diff --git a/go.mod b/go.mod index 688e8fe1d..8b3743df3 100644 --- a/go.mod +++ b/go.mod @@ -33,8 +33,8 @@ replace github.com/hashicorp/go-retryablehttp => github.com/hashicorp/go-retryab require ( github.com/MakeNowJust/heredoc/v2 v2.0.1 - github.com/chromedp/cdproto v0.0.0-20241003230502-a4a8f7c660df - github.com/chromedp/chromedp v0.10.0 + github.com/chromedp/cdproto v0.0.0-20241014181340-cb3a7a1d51d7 + github.com/chromedp/chromedp v0.11.0 github.com/coreos/go-oidc/v3 v3.11.0 github.com/coreos/go-semver v0.3.1 github.com/creack/pty v1.1.23 @@ -64,7 +64,7 @@ require ( github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 - github.com/tdewolff/minify/v2 v2.20.37 + github.com/tdewolff/minify/v2 v2.21.0 go.uber.org/mock v0.4.0 go.uber.org/zap v1.27.0 golang.org/x/crypto v0.28.0 @@ -162,7 +162,7 @@ require ( github.com/spf13/viper v1.16.0 // indirect github.com/stoewer/go-strcase v1.2.0 // indirect github.com/subosito/gotenv v1.4.2 // indirect - github.com/tdewolff/parse/v2 v2.7.15 // indirect + github.com/tdewolff/parse/v2 v2.7.17 // indirect github.com/x448/float16 v0.8.4 // indirect go.etcd.io/etcd/api/v3 v3.5.14 // indirect go.etcd.io/etcd/client/pkg/v3 v3.5.14 // indirect diff --git a/go.sum b/go.sum index deee538e2..a3edc8ffa 100644 --- a/go.sum +++ b/go.sum @@ -64,11 +64,11 @@ github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/chromedp/cdproto v0.0.0-20240801214329-3f85d328b335/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= -github.com/chromedp/cdproto v0.0.0-20241003230502-a4a8f7c660df h1:cbtSn19AtqQha1cxmP2Qvgd3fFMz51AeAEKLJMyEUhc= github.com/chromedp/cdproto v0.0.0-20241003230502-a4a8f7c660df/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= -github.com/chromedp/chromedp v0.10.0 h1:bRclRYVpMm/UVD76+1HcRW9eV3l58rFfy7AdBvKab1E= -github.com/chromedp/chromedp v0.10.0/go.mod h1:ei/1ncZIqXX1YnAYDkxhD4gzBgavMEUu7JCKvztdomE= +github.com/chromedp/cdproto v0.0.0-20241014181340-cb3a7a1d51d7 h1:VDBgUGgdCBw9lTKwp0KPExhnqmGfGVJQTER2MehoICk= +github.com/chromedp/cdproto v0.0.0-20241014181340-cb3a7a1d51d7/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= +github.com/chromedp/chromedp v0.11.0 h1:1PT6O4g39sBAFjlljIHTpxmCSk8meeYL6+R+oXH4bWA= +github.com/chromedp/chromedp v0.11.0/go.mod h1:jsD7OHrX0Qmskqb5Y4fn4jHnqquqW22rkMFgKbECsqg= github.com/chromedp/sysutil v1.0.0 h1:+ZxhTpfpZlmchB58ih/LBHX52ky7w2VhQVKQMucy3Ic= github.com/chromedp/sysutil v1.0.0/go.mod h1:kgWmDdq8fTzXYcKIBqIYvRRTnYb9aNS9moAV0xufSww= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= @@ -560,10 +560,10 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/subosito/gotenv v1.4.2 h1:X1TuBLAMDFbaTAChgCBLu3DU3UPyELpnF2jjJ2cz/S8= github.com/subosito/gotenv v1.4.2/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0= -github.com/tdewolff/minify/v2 v2.20.37 h1:Q97cx4STXCh1dlWDlNHZniE8BJ2EBL0+2b0n92BJQhw= -github.com/tdewolff/minify/v2 v2.20.37/go.mod h1:L1VYef/jwKw6Wwyk5A+T0mBjjn3mMPgmjjA688RNsxU= -github.com/tdewolff/parse/v2 v2.7.15 h1:hysDXtdGZIRF5UZXwpfn3ZWRbm+ru4l53/ajBRGpCTw= -github.com/tdewolff/parse/v2 v2.7.15/go.mod h1:3FbJWZp3XT9OWVN3Hmfp0p/a08v4h8J9W1aghka0soA= +github.com/tdewolff/minify/v2 v2.21.0 h1:nAPP1UVx0aK1xsQh/JiG3xyEnnqWw+agPstn+V6Pkto= +github.com/tdewolff/minify/v2 v2.21.0/go.mod h1:hGcthJ6Vj51NG+9QRIfN/DpWj5loHnY3bfhThzWWq08= +github.com/tdewolff/parse/v2 v2.7.17 h1:uC10p6DaQQORDy72eaIyD+AvAkaIUOouQ0nWp4uD0D0= +github.com/tdewolff/parse/v2 v2.7.17/go.mod h1:3FbJWZp3XT9OWVN3Hmfp0p/a08v4h8J9W1aghka0soA= github.com/tdewolff/test v1.0.11-0.20231101010635-f1265d231d52/go.mod h1:6DAvZliBAAnD7rhVgwaM7DE5/d9NMOAJ09SqYqeK4QE= github.com/tdewolff/test v1.0.11-0.20240106005702-7de5f7df4739 h1:IkjBCtQOOjIn03u/dMQK9g+Iw9ewps4mCl1nB8Sscbo= github.com/tdewolff/test v1.0.11-0.20240106005702-7de5f7df4739/go.mod h1:XPuWBzvdUzhCuxWO1ojpXsyzsA5bFoS3tO/Q3kFuTG8= @@ -851,7 +851,6 @@ golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 338f212b1..0f8f7c045 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -34,7 +34,7 @@ clean_kind=no api_group_suffix="pinniped.dev" # same default as in the values.yaml ytt file dockerfile_path="" get_active_directory_vars="" # specify a filename for a script to get AD related env variables -get_github_vars="" # specify a filename for a script to get GitHub related env variables +get_github_vars="" # specify a filename for a script to get GitHub related env variables alternate_deploy="undefined" pre_install="undefined" @@ -319,6 +319,15 @@ service_https_nodeport_nodeport: $service_https_nodeport_nodeport service_https_clusterip_port: $service_https_clusterip_port EOF +if [[ "${FIREWALL_IDPS:-no}" == "yes" ]]; then + # Configure the web proxy on the Supervisor pods. Note that .svc and .cluster.local are not included, + # so requests for things like dex.tools.svc.cluster.local will go through the web proxy. + cat <>"$data_values_file" +https_proxy: "http://proxy.tools.svc.cluster.local:3128" +no_proxy: "\$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost" +EOF +fi + if [ "$alternate_deploy" != "undefined" ]; then log_note "The Pinniped Supervisor will be deployed with $alternate_deploy pinniped-supervisor $tag $registry_with_port $repo $data_values_file ..." $alternate_deploy pinniped-supervisor "$tag" $registry_with_port $repo $data_values_file @@ -338,7 +347,7 @@ manifest=/tmp/pinniped-concierge.yaml data_values_file=/tmp/concierge-values.yml concierge_app_name="pinniped-concierge" concierge_namespace="concierge" -webhook_url="https://local-user-authenticator.local-user-authenticator.svc/authenticate" +webhook_url="https://local-user-authenticator.local-user-authenticator.svc.cluster.local/authenticate" discovery_url="$(TERM=dumb kubectl cluster-info | awk '/master|control plane/ {print $NF}')" concierge_custom_labels="{myConciergeCustomLabelName: myConciergeCustomLabelValue}" log_level="debug" @@ -354,6 +363,16 @@ image_tag: $tag discovery_url: $discovery_url EOF +if [[ "${FIREWALL_IDPS:-no}" == "yes" ]]; then + # Configure the web proxy on the Concierge pods. Note that .svc and .cluster.local are not included, + # so requests for things like pinniped-supervisor-clusterip.supervisor.svc.cluster.local and + # local-user-authenticator.local-user-authenticator.svc.cluster.local will go through the web proxy. + cat <>"$data_values_file" +https_proxy: "http://proxy.tools.svc.cluster.local:3128" +no_proxy: "\$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost" +EOF +fi + if [ "$alternate_deploy" != "undefined" ]; then log_note "The Pinniped Concierge will be deployed with $alternate_deploy pinniped-concierge $tag $registry_with_port $repo $data_values_file ..." $alternate_deploy pinniped-concierge "$tag" $registry_with_port $repo $data_values_file @@ -366,6 +385,77 @@ else popd >/dev/null fi +# +# Now that the everything is deployed, optionally firewall the Dex server, the local user authenticator server, +# and the GitHub API so that the Supervisor and Concierge cannot reach them directly. However, the Squid +# proxy server can reach them all, so the Supervisor and Concierge can reach them through the proxy. +# +if [[ "${FIREWALL_IDPS:-no}" == "yes" ]]; then + log_note "Setting up firewalls for the Supervisor and Concierge's outgoing TCP/UDP network traffic..." + cat <$webhookcachefiller.(*webhookCacheFillerController).syncIndividualWebhookAuthenticator","message":"cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations","webhookAuthenticator":"test-name","endpoint":"%s"}`, goodWebhookAuthenticatorSpecWithCA.Endpoint), + fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:$webhookcachefiller.(*webhookCacheFillerController).updateStatus","message":"webhookauthenticator status successfully updated","webhookAuthenticator":"test-name","endpoint":"%s","phase":"Ready"}`, goodWebhookAuthenticatorSpecWithCA.Endpoint), + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + Generation: 1234, + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ // updates the status to ready + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 1234), + []metav1.Condition{ + happyWebhookConnectionValidWithoutDialingDueToProxy(frozenMetav1Now, 1234), + }, + ), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantNamesOfWebhookAuthenticatorsInCache: []string{"test-name"}, // keeps the old entry in the cache + }, { name: "Sync: valid WebhookAuthenticator with CA: will complete sync loop successfully with success conditions and ready phase", webhookAuthenticators: []runtime.Object{ @@ -1021,6 +1115,18 @@ func TestController(t *testing.T) { Spec: goodWebhookAuthenticatorSpecWithCA, }, }, + proxyDetector: func(t *testing.T) proxydetect.ProxyDetect { + // Detect that a proxy is not required for the webhook host. + fakeProxyDetect := fakeproxydetect.New(false, nil) + t.Cleanup(func() { + require.Equal(t, 1, fakeProxyDetect.NumberOfInvocations()) + require.Equal(t, + strings.TrimPrefix(goodWebhookAuthenticatorSpecWithCA.Endpoint, "https://"), + fakeProxyDetect.ReceivedHostDuringMostRecentInvocation(), + ) + }) + return fakeProxyDetect + }, wantLogLines: []string{ fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:$webhookcachefiller.(*webhookCacheFillerController).updateStatus","message":"webhookauthenticator status successfully updated","webhookAuthenticator":"test-name","endpoint":"%s","phase":"Ready"}`, goodWebhookDefaultServingCertEndpoint), fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:$webhookcachefiller.(*webhookCacheFillerController).syncIndividualWebhookAuthenticator","message":"added or updated webhook authenticator in cache","webhookAuthenticator":"test-name","endpoint":"%s","isOverwrite":false}`, goodWebhookDefaultServingCertEndpoint), @@ -1045,6 +1151,110 @@ func TestController(t *testing.T) { }, wantNamesOfWebhookAuthenticatorsInCache: []string{"test-name"}, }, + { + name: "Sync: valid WebhookAuthenticator when webhook host will use web proxy: will complete sync loop successfully with success conditions and ready phase", + webhookAuthenticators: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + }, + }, + proxyDetector: func(t *testing.T) proxydetect.ProxyDetect { + // Detect that a proxy is required for the webhook host. + fakeProxyDetect := fakeproxydetect.New(true, nil) + t.Cleanup(func() { + require.Equal(t, 1, fakeProxyDetect.NumberOfInvocations()) + require.Equal(t, + strings.TrimPrefix(goodWebhookAuthenticatorSpecWithCA.Endpoint, "https://"), + fakeProxyDetect.ReceivedHostDuringMostRecentInvocation(), + ) + }) + return fakeProxyDetect + }, + wantLogLines: []string{ + fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:$webhookcachefiller.(*webhookCacheFillerController).updateStatus","message":"webhookauthenticator status successfully updated","webhookAuthenticator":"test-name","endpoint":"%s","phase":"Ready"}`, goodWebhookDefaultServingCertEndpoint), + fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:$webhookcachefiller.(*webhookCacheFillerController).syncIndividualWebhookAuthenticator","message":"added or updated webhook authenticator in cache","webhookAuthenticator":"test-name","endpoint":"%s","isOverwrite":false}`, goodWebhookDefaultServingCertEndpoint), + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + []metav1.Condition{ + happyWebhookConnectionValidWithoutDialingDueToProxy(frozenMetav1Now, 0), + }, + ), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantNamesOfWebhookAuthenticatorsInCache: []string{"test-name"}, + }, + { + name: "Sync: valid WebhookAuthenticator when error while checking if webhook host will use web proxy: will complete sync loop successfully with error conditions and error phase", + webhookAuthenticators: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + }, + }, + proxyDetector: func(t *testing.T) proxydetect.ProxyDetect { + // Return a fake error when trying to determine if a proxy is needed for the webhook host. + fakeProxyDetect := fakeproxydetect.New(false, errors.New("fake proxy detector error")) + t.Cleanup(func() { + require.Equal(t, 1, fakeProxyDetect.NumberOfInvocations()) + require.Equal(t, + strings.TrimPrefix(goodWebhookAuthenticatorSpecWithCA.Endpoint, "https://"), + fakeProxyDetect.ReceivedHostDuringMostRecentInvocation(), + ) + }) + return fakeProxyDetect + }, + wantLogLines: []string{ + fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:$webhookcachefiller.(*webhookCacheFillerController).syncIndividualWebhookAuthenticator","message":"invalid webhook authenticator","webhookAuthenticator":"test-name","endpoint":"%s","removedFromCache":false}`, goodWebhookDefaultServingCertEndpoint), + fmt.Sprintf(`{"level":"debug","timestamp":"2099-08-08T13:57:36.123456Z","logger":"webhookcachefiller-controller","caller":"webhookcachefiller/webhookcachefiller.go:$webhookcachefiller.(*webhookCacheFillerController).updateStatus","message":"webhookauthenticator status successfully updated","webhookAuthenticator":"test-name","endpoint":"%s","phase":"Error"}`, goodWebhookDefaultServingCertEndpoint), + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + []metav1.Condition{ + sadEndpointURLValidProxyDetectErr("spec.endpoint URL error: fake proxy detector error", frozenMetav1Now, 0), + unknownWebhookConnectionValid(frozenMetav1Now, 0), + unknownAuthenticatorValid(frozenMetav1Now, 0), + sadReadyCondition(frozenMetav1Now, 0), + }, + ), + Phase: "Error", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + }, { name: "Sync: valid WebhookAuthenticator with IPV6 and CA: will complete sync loop successfully with success conditions and ready phase", webhookAuthenticators: []runtime.Object{ @@ -1925,6 +2135,13 @@ func TestController(t *testing.T) { tt.cache(t, cache) } + if tt.proxyDetector == nil { + // By default, detect that a proxy is not required for any webhook hosts. + tt.proxyDetector = func(t *testing.T) proxydetect.ProxyDetect { + return fakeproxydetect.New(false, nil) + } + } + controller := New( "concierge", // namespace for controller cache, @@ -1935,7 +2152,9 @@ func TestController(t *testing.T) { controllerlib.WithInformer, frozenClock, logger, - ptls.NewDialer()) + ptls.NewDialer(), + tt.proxyDetector(t), + ) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -2179,7 +2398,9 @@ func TestControllerFilterSecret(t *testing.T) { observableInformers.WithInformer, frozenClock, logger, - ptls.NewDialer()) + ptls.NewDialer(), + fakeproxydetect.New(false, nil), + ) unrelated := &corev1.Secret{} filter := observableInformers.GetFilterForInformer(secretInformer) @@ -2241,7 +2462,9 @@ func TestControllerFilterConfigMap(t *testing.T) { observableInformers.WithInformer, frozenClock, logger, - ptls.NewDialer()) + ptls.NewDialer(), + fakeproxydetect.New(false, nil), + ) unrelated := &corev1.ConfigMap{} filter := observableInformers.GetFilterForInformer(configMapInformer) diff --git a/internal/controller/conditionsutil/conditions_util.go b/internal/controller/conditionsutil/conditions_util.go index ee2f2434f..6416b8a88 100644 --- a/internal/controller/conditionsutil/conditions_util.go +++ b/internal/controller/conditionsutil/conditions_util.go @@ -15,11 +15,10 @@ import ( // Some common reasons and messages shared by conditions of various resources. const ( - ReasonSuccess = "Success" - ReasonNotReady = "NotReady" - ReasonUnableToValidate = "UnableToValidate" - ReasonUnableToDialServer = "UnableToDialServer" - ReasonInvalidIssuerURL = "InvalidIssuerURL" + ReasonSuccess = "Success" + ReasonNotReady = "NotReady" + ReasonUnableToValidate = "UnableToValidate" + ReasonInvalidIssuerURL = "InvalidIssuerURL" MessageUnableToValidate = "unable to validate; see other conditions for details" ) diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index 71cd980f3..56fd12146 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -33,7 +33,6 @@ import ( "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/federationdomain/upstreamprovider" "go.pinniped.dev/internal/net/phttp" @@ -58,11 +57,19 @@ const ( GitHubConnectionValid string = "GitHubConnectionValid" ClaimsValid string = "ClaimsValid" - reasonInvalid = "Invalid" - reasonInvalidHost = "InvalidHost" + reasonInvalid = "Invalid" + reasonInvalidHost = "InvalidHost" + reasonUnableToMakeRequest = "UnableToMakeRequest" apiDotGithubDotCom = "api.github.com" githubDotCom = "github.com" + + headerNameAPIVersion = "X-GitHub-Api-Version" + headerValueAPIVersion2022 = "2022-11-28" + headerNameUserAgent = "User-Agent" + headerValueUserAgentConcierge = "pinniped-concierge/v1" + headerNameAccept = "Accept" + headerValueAcceptGitHubJSON = "application/vnd.github+json" ) // UpstreamGitHubIdentityProviderICache is a thread safe cache that holds a list of validated upstream GitHub IDP configurations. @@ -107,6 +114,8 @@ func NewGitHubValidatedAPICache(cache *cache.Expiring) GitHubValidatedAPICacheI return &GitHubValidatedAPICache{cache: cache} } +type ProbeURLFunc func(ctx context.Context, client *http.Client, url string) error + type gitHubWatcherController struct { namespace string cache UpstreamGitHubIdentityProviderICache @@ -116,7 +125,7 @@ type gitHubWatcherController struct { secretInformer corev1informers.SecretInformer configMapInformer corev1informers.ConfigMapInformer clock clock.Clock - dialer ptls.Dialer + probeURLFunc ProbeURLFunc validatedCache GitHubValidatedAPICacheI } @@ -131,7 +140,7 @@ func New( log plog.Logger, withInformer pinnipedcontroller.WithInformerOptionFunc, clock clock.Clock, - dialer ptls.Dialer, + probeURLFunc ProbeURLFunc, validatedCache *cache.Expiring, ) controllerlib.Controller { c := gitHubWatcherController{ @@ -143,7 +152,7 @@ func New( secretInformer: secretInformer, configMapInformer: configMapInformer, clock: clock, - dialer: dialer, + probeURLFunc: probeURLFunc, validatedCache: NewGitHubValidatedAPICache(validatedCache), } @@ -299,7 +308,7 @@ func validateOrganizationsPolicy(organizationsSpec *idpv1alpha1.GitHubOrganizati func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx context.Context, upstream *idpv1alpha1.GitHubIdentityProvider) ( *upstreamgithub.Provider, // If validated, returns the config - error, // This error will only refer to programmatic errors such as inability to perform a Dial or dereference a pointer, not configuration errors + error, // This error will only refer to programmatic errors such as inability to perform a connection probe or dereference a pointer, not configuration errors ) { conditions := make([]*metav1.Condition, 0) applicationErrors := make([]error, 0) @@ -365,7 +374,7 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contex upstreamgithub.ProviderConfig{ Name: upstream.Name, ResourceUID: upstream.UID, - APIBaseURL: apiBaseUrl(apiHostPort), + APIBaseURL: apiBaseURL(apiHostPort), GroupNameAttribute: groupNameAttribute, UsernameAttribute: usernameAttribute, OAuth2Config: &oauth2.Config{ @@ -388,7 +397,7 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contex return provider, utilerrors.NewAggregate(applicationErrors) } -func apiBaseUrl(apiHostPort *endpointaddr.HostPort) string { +func apiBaseURL(apiHostPort *endpointaddr.HostPort) string { endpoint := hostPortForHTTPS(apiHostPort) if strings.ToLower(apiHostPort.Host) == apiDotGithubDotCom { @@ -471,19 +480,33 @@ func (c *gitHubWatcherController) validateGitHubConnection( apiAddress := apiHostPort.Endpoint() - if !c.validatedCache.IsValid(apiAddress, caBundle.Hash()) { - dialCtx, dialCancel := context.WithTimeout(ctx, 30*time.Second) - defer dialCancel() + // Note that this client already has some reasonable timeouts configured on it. + httpClient := phttp.Default(caBundle.CertPool()) - tlsDialErr := c.dialer.IsReachableAndTLSValidationSucceeds(dialCtx, apiAddress, caBundle.CertPool(), c.log) - if tlsDialErr != nil { + if !c.validatedCache.IsValid(apiAddress, caBundle.Hash()) { + // In order to provide feedback on the configured host and CA bundle, probe the configured server by using + // a real HTTPS request, so we can also respect the pod's proxy settings during the request, if they are present. + // To make a real request, we need to choose a real endpoint. + // According to the GitHub API docs for your getting your rate limit status, "Accessing this endpoint does not + // count against your REST API rate limit" and you do not need to be authenticated to use this endpoint. + // See https://docs.github.com/en/rest/rate-limit/rate-limit?apiVersion=2022-11-28#get-rate-limit-status-for-the-authenticated-user. + // Note that like other APIs, the URL path is different for GitHub Enterprise Server (https://HOSTNAME/api/v3/rate_limit) + // versus Enterprise Cloud or Public (https://api.github.com/rate_limit). + // The docs also say that "Unauthenticated requests are associated with the originating IP address" and + // "The primary rate limit for unauthenticated requests is 60 requests per hour." + // So we'll use this endpoint in hopes that we can avoid impacting the rate limit for our originating IP, + // which could potentially be shared with other apps. + rateLimitAPIURL := fmt.Sprintf("%s/rate_limit", apiBaseURL(apiHostPort)) + + err := c.probeURLFunc(ctx, httpClient, rateLimitAPIURL) + if err != nil { return &metav1.Condition{ Type: GitHubConnectionValid, Status: metav1.ConditionFalse, - Reason: conditionsutil.ReasonUnableToDialServer, - Message: fmt.Sprintf("cannot dial %q for spec.githubAPI.host (%q): %s", - apiAddress, *specifiedHost, buildDialErrorMessage(tlsDialErr)), - }, nil, tlsDialErr + Reason: reasonUnableToMakeRequest, + Message: fmt.Sprintf("cannot make connection probe request for spec.githubAPI.host (%q): %s", + *specifiedHost, buildProbeErrorMessage(err)), + }, nil, err } } @@ -493,18 +516,49 @@ func (c *gitHubWatcherController) validateGitHubConnection( Type: GitHubConnectionValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: fmt.Sprintf("dialed %q for spec.githubAPI.host (%q): host is reachable and TLS verification succeeds", + Message: fmt.Sprintf("probed connection to %q for spec.githubAPI.host (%q): host is reachable and TLS verification succeeds", apiAddress, *specifiedHost), - }, phttp.Default(caBundle.CertPool()), nil + }, httpClient, nil } -// buildDialErrorMessage standardizes DNS error messages that appear differently on different platforms, so that tests and log grepping is uniform. -func buildDialErrorMessage(tlsDialErr error) string { - reason := tlsDialErr.Error() +// ProbeURL is the production code for how to probe a GitHub URL to test our connection to the GitHub API. +// It can be replaced via constructor injection for testing. Implements ProbeURLFunc. +func ProbeURL(ctx context.Context, client *http.Client, url string) error { + probeRequestCtx, probeRequestCancel := context.WithTimeout(ctx, 30*time.Second) + defer probeRequestCancel() + + probeRequest, err := http.NewRequestWithContext(probeRequestCtx, http.MethodGet, url, nil) + if err != nil { + // Shouldn't really get here as long as the URL is valid. + return err + } + + // GitHub API docs say that these headers are required for all requests. + // See https://docs.github.com/en/rest/using-the-rest-api/getting-started-with-the-rest-api?apiVersion=2022-11-28#headers. + probeRequest.Header.Set(headerNameAPIVersion, headerValueAPIVersion2022) + probeRequest.Header.Set(headerNameUserAgent, headerValueUserAgentConcierge) + probeRequest.Header.Set(headerNameAccept, headerValueAcceptGitHubJSON) + + probeResponse, err := client.Do(probeRequest) + if err != nil { + return err + } + + // Don't care what the response was, even if it was an error status, as long as we were able to connect + // successfully to the specified host using the specified CA bundle and the pod's proxy env var settings. + _ = probeResponse.Body.Close() + + return nil +} + +// buildProbeErrorMessage standardizes DNS error messages that appear differently on different platforms, +// so that tests and log grepping is uniform. +func buildProbeErrorMessage(err error) string { + reason := err.Error() var opError *net.OpError var dnsError *net.DNSError - if errors.As(tlsDialErr, &opError) && errors.As(tlsDialErr, &dnsError) { + if errors.As(err, &opError) && errors.As(err, &dnsError) { dnsError.Server = "" opError.Err = dnsError return opError.Error() diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index 74e5e6151..51b273dd0 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -40,7 +40,6 @@ import ( "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" - "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" "go.pinniped.dev/internal/federationdomain/upstreamprovider" "go.pinniped.dev/internal/net/phttp" @@ -61,32 +60,55 @@ var ( githubIDPKind = idpv1alpha1.SchemeGroupVersion.WithKind("GitHubIdentityProvider") ) -type fakeGithubDialer struct { - t *testing.T - realAddress string - realCertPool *x509.CertPool +func fakeGithubProbeFuncProbesOtherAddress(t *testing.T, realAddress string) ProbeURLFunc { + return func(ctx context.Context, client *http.Client, url string) error { + t.Helper() + require.Equal(t, "https://api.github.com/rate_limit", url) + r, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://"+realAddress+"/rate_limit", nil) + require.NoError(t, err) + resp, err := client.Do(r) + _ = resp.Body.Close() + return err + } } -func (f fakeGithubDialer) IsReachableAndTLSValidationSucceeds(ctx context.Context, address string, _ *x509.CertPool, logger plog.Logger) error { - require.Equal(f.t, "api.github.com:443", address) - - return ptls.NewDialer().IsReachableAndTLSValidationSucceeds(ctx, f.realAddress, f.realCertPool, logger) +func fakeGithubProbeFuncDisallowsAllProbes(t *testing.T) ProbeURLFunc { + return func(ctx context.Context, client *http.Client, url string) error { + t.Helper() + t.Errorf("this test should not perform any connection probe") + t.FailNow() + return nil + } } -var _ ptls.Dialer = (*fakeGithubDialer)(nil) +// TestProbeURL tests the production version of the ProbeURLFunc type. +func TestProbeURL(t *testing.T) { + serverEndpointReached := false + testServer, testServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, []string{"2022-11-28"}, r.Header.Values("X-GitHub-Api-Version")) + require.Equal(t, []string{"pinniped-concierge/v1"}, r.Header.Values("User-Agent")) + require.Equal(t, []string{"application/vnd.github+json"}, r.Header.Values("Accept")) + serverEndpointReached = true + }), nil) -type allowNoDials struct { - t *testing.T + testServerCertPool := x509.NewCertPool() + testServerCertPool.AppendCertsFromPEM(testServerCA) + httpClient := phttp.Default(testServerCertPool) + + // Happy path. + err := ProbeURL(context.Background(), httpClient, testServer.URL) + require.NoError(t, err) + require.True(t, serverEndpointReached) + + // Invalid URL. + err = ProbeURL(context.Background(), httpClient, "https://invalid hostname") + require.EqualError(t, err, `parse "https://invalid hostname": invalid character " " in host name`) + + // Did not trust server's CA, as an example of a failed connection. + err = ProbeURL(context.Background(), phttp.Default(nil), testServer.URL) + require.EqualError(t, err, fmt.Sprintf("Get %q: tls: failed to verify certificate: x509: certificate signed by unknown authority", testServer.URL)) } -func (f allowNoDials) IsReachableAndTLSValidationSucceeds(_ context.Context, _ string, _ *x509.CertPool, _ plog.Logger) error { - f.t.Errorf("this test should not perform dial") - f.t.FailNow() - return nil -} - -var _ ptls.Dialer = (*allowNoDials)(nil) - func TestController(t *testing.T) { require.Equal(t, 6, countExpectedConditions) @@ -215,8 +237,6 @@ func TestController(t *testing.T) { } buildHostValidTrue := func(t *testing.T, host string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: HostValid, Status: metav1.ConditionTrue, @@ -228,8 +248,6 @@ func TestController(t *testing.T) { } buildHostValidFalse := func(t *testing.T, host, message string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: HostValid, Status: metav1.ConditionFalse, @@ -240,8 +258,6 @@ func TestController(t *testing.T) { } } buildTLSConfigurationValidTrueWithMsg := func(t *testing.T, msg string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: TLSConfigurationValid, Status: metav1.ConditionTrue, @@ -253,13 +269,10 @@ func TestController(t *testing.T) { } buildTLSConfigurationValidTrue := func(t *testing.T) metav1.Condition { - t.Helper() return buildTLSConfigurationValidTrueWithMsg(t, "using configured CA bundle") } buildTLSConfigurationValidFalse := func(t *testing.T, message string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: TLSConfigurationValid, Status: metav1.ConditionFalse, @@ -271,8 +284,6 @@ func TestController(t *testing.T) { } buildOrganizationsPolicyValidTrue := func(t *testing.T, policy idpv1alpha1.GitHubAllowedAuthOrganizationsPolicy) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: OrganizationsPolicyValid, Status: metav1.ConditionTrue, @@ -284,8 +295,6 @@ func TestController(t *testing.T) { } buildOrganizationsPolicyValidFalse := func(t *testing.T, message string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: OrganizationsPolicyValid, Status: metav1.ConditionFalse, @@ -297,8 +306,6 @@ func TestController(t *testing.T) { } buildClientCredentialsSecretValidTrue := func(t *testing.T, secretName string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: ClientCredentialsSecretValid, Status: metav1.ConditionTrue, @@ -310,8 +317,6 @@ func TestController(t *testing.T) { } buildClientCredentialsSecretValidFalse := func(t *testing.T, prefix, secretName, namespace, reason string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: ClientCredentialsSecretValid, Status: metav1.ConditionFalse, @@ -328,8 +333,6 @@ func TestController(t *testing.T) { } buildClaimsValidatedTrue := func(t *testing.T) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: ClaimsValid, Status: metav1.ConditionTrue, @@ -341,8 +344,6 @@ func TestController(t *testing.T) { } buildClaimsValidatedFalse := func(t *testing.T, message string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: ClaimsValid, Status: metav1.ConditionFalse, @@ -353,35 +354,29 @@ func TestController(t *testing.T) { } } - buildGitHubConnectionValidTrue := func(t *testing.T, hostDialed string, hostSpecified string) metav1.Condition { - t.Helper() - + buildGitHubConnectionValidTrue := func(t *testing.T, hostProbed string, hostSpecified string) metav1.Condition { return metav1.Condition{ Type: GitHubConnectionValid, Status: metav1.ConditionTrue, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, Reason: conditionsutil.ReasonSuccess, - Message: fmt.Sprintf("dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds", hostDialed, hostSpecified), + Message: fmt.Sprintf("probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds", hostProbed, hostSpecified), } } buildGitHubConnectionValidFalse := func(t *testing.T, message string) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: GitHubConnectionValid, Status: metav1.ConditionFalse, ObservedGeneration: wantObservedGeneration, LastTransitionTime: wantLastTransitionTime, - Reason: "UnableToDialServer", + Reason: "UnableToMakeRequest", Message: message, } } buildGitHubConnectionValidUnknown := func(t *testing.T) metav1.Condition { - t.Helper() - return metav1.Condition{ Type: GitHubConnectionValid, Status: metav1.ConditionUnknown, @@ -416,8 +411,12 @@ func TestController(t *testing.T) { return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"TLSConfigurationValid","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, message) } - buildLogForUpdatingGitHubConnectionValid := func(name, status, reason, messageFmt, dialHost, specHost string) string { - return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"GitHubConnectionValid","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, fmt.Sprintf(messageFmt, dialHost, specHost)) + buildLogForUpdatingGitHubConnectionValid := func(name, status, reason, messageFmt, probedHost, specHost string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"GitHubConnectionValid","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, fmt.Sprintf(messageFmt, probedHost, specHost)) + } + + buildLogForUpdatingGitHubConnectionInvalid := func(name, msg string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"GitHubConnectionValid","status":"%s","reason":"%s","message":"%s"}`, name, "False", "UnableToMakeRequest", msg) } buildLogForUpdatingGitHubConnectionValidUnknown := func(name string) string { @@ -432,7 +431,7 @@ func TestController(t *testing.T) { name string githubIdentityProviders []runtime.Object secretsAndConfigMaps []runtime.Object - mockDialer func(*testing.T) ptls.Dialer + mockProbeURLFunc func(t *testing.T) ProbeURLFunc preexistingValidatedCache []GitHubValidatedAPICacheKey wantErr string wantLogs []string @@ -504,7 +503,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Ready"), }, }, @@ -566,7 +565,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("minimal-idp-name", "Ready"), }, }, @@ -581,14 +580,8 @@ func TestController(t *testing.T) { return githubIDP }(), }, - mockDialer: func(t *testing.T) ptls.Dialer { - t.Helper() - - return &fakeGithubDialer{ - t: t, - realAddress: goodServerDomain, - realCertPool: goodServerCertPool, - } + mockProbeURLFunc: func(t *testing.T) ProbeURLFunc { + return fakeGithubProbeFuncProbesOtherAddress(t, goodServerDomain) }, wantResultingCache: []*upstreamgithub.ProviderConfig{ { @@ -647,7 +640,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "github.com"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "github.com"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "github.com"), buildLogForUpdatingPhase("minimal-idp-name", "Ready"), }, }, @@ -662,14 +655,8 @@ func TestController(t *testing.T) { return githubIDP }(), }, - mockDialer: func(t *testing.T) ptls.Dialer { - t.Helper() - - return &fakeGithubDialer{ - t: t, - realAddress: goodServerDomain, - realCertPool: goodServerCertPool, - } + mockProbeURLFunc: func(t *testing.T) ProbeURLFunc { + return fakeGithubProbeFuncProbesOtherAddress(t, goodServerDomain) }, wantResultingCache: []*upstreamgithub.ProviderConfig{ { @@ -728,7 +715,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "GitHub.com"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "GitHub.com"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "GitHub.com"), buildLogForUpdatingPhase("minimal-idp-name", "Ready"), }, }, @@ -743,14 +730,8 @@ func TestController(t *testing.T) { return githubIDP }(), }, - mockDialer: func(t *testing.T) ptls.Dialer { - t.Helper() - - return &fakeGithubDialer{ - t: t, - realAddress: goodServerDomain, - realCertPool: goodServerCertPool, - } + mockProbeURLFunc: func(t *testing.T) ProbeURLFunc { + return fakeGithubProbeFuncProbesOtherAddress(t, goodServerDomain) }, wantResultingCache: []*upstreamgithub.ProviderConfig{ { @@ -809,7 +790,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "api.github.com"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "api.github.com"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "api.github.com"), buildLogForUpdatingPhase("minimal-idp-name", "Ready"), }, }, @@ -824,14 +805,8 @@ func TestController(t *testing.T) { return githubIDP }(), }, - mockDialer: func(t *testing.T) ptls.Dialer { - t.Helper() - - return &fakeGithubDialer{ - t: t, - realAddress: goodServerDomain, - realCertPool: goodServerCertPool, - } + mockProbeURLFunc: func(t *testing.T) ProbeURLFunc { + return fakeGithubProbeFuncProbesOtherAddress(t, goodServerDomain) }, wantResultingCache: []*upstreamgithub.ProviderConfig{ { @@ -890,7 +865,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "github.com:443"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "github.com:443"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "github.com:443"), buildLogForUpdatingPhase("minimal-idp-name", "Ready"), }, }, @@ -905,14 +880,8 @@ func TestController(t *testing.T) { return githubIDP }(), }, - mockDialer: func(t *testing.T) ptls.Dialer { - t.Helper() - - return &fakeGithubDialer{ - t: t, - realAddress: goodServerDomain, - realCertPool: goodServerCertPool, - } + mockProbeURLFunc: func(t *testing.T) ProbeURLFunc { + return fakeGithubProbeFuncProbesOtherAddress(t, goodServerDomain) }, wantResultingCache: []*upstreamgithub.ProviderConfig{ { @@ -971,7 +940,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "api.GitHub.com:443"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "api.GitHub.com:443"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, "api.github.com:443", "api.GitHub.com:443"), buildLogForUpdatingPhase("minimal-idp-name", "Ready"), }, }, @@ -1048,7 +1017,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, goodServerIPv6Domain), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, goodServerIPv6Domain, goodServerIPv6Domain), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, goodServerIPv6Domain, goodServerIPv6Domain), buildLogForUpdatingPhase("minimal-idp-name", "Ready"), }, }, @@ -1208,7 +1177,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("invalid-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("invalid-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("invalid-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("invalid-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("invalid-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("invalid-idp-name", "Error"), buildLogForUpdatingClientCredentialsSecretValid("other-idp-name", "True", "Success", `clientID and clientSecret have been read from spec.client.SecretName (\"other-secret-name\")`), @@ -1216,7 +1185,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("other-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("other-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("other-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("other-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("other-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("other-idp-name", "Ready"), buildLogForUpdatingClientCredentialsSecretValid("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), @@ -1224,7 +1193,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Ready"), }, }, @@ -1379,7 +1348,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("idp-with-tls-in-config-map", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("idp-with-tls-in-config-map", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("idp-with-tls-in-config-map", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("idp-with-tls-in-config-map", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("idp-with-tls-in-config-map", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("idp-with-tls-in-config-map", "Ready"), buildLogForUpdatingClientCredentialsSecretValid("idp-with-tls-in-secret", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), @@ -1387,7 +1356,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("idp-with-tls-in-secret", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("idp-with-tls-in-secret", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("idp-with-tls-in-secret", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("idp-with-tls-in-secret", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("idp-with-tls-in-secret", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("idp-with-tls-in-secret", "Ready"), }, }, @@ -1395,11 +1364,7 @@ func TestController(t *testing.T) { name: "happy path with previously validated address/CA Bundle does not validate again", secretsAndConfigMaps: []runtime.Object{goodClientCredentialsSecret}, githubIdentityProviders: []runtime.Object{validFilledOutIDP}, - mockDialer: func(t *testing.T) ptls.Dialer { - t.Helper() - - return &allowNoDials{t: t} - }, + mockProbeURLFunc: fakeGithubProbeFuncDisallowsAllProbes, preexistingValidatedCache: []GitHubValidatedAPICacheKey{ { address: goodServerDomain, @@ -1458,7 +1423,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Ready"), }, }, @@ -1777,7 +1742,7 @@ func TestController(t *testing.T) { Conditions: []metav1.Condition{ buildClaimsValidatedTrue(t), buildClientCredentialsSecretValidTrue(t, validMinimalIDP.Spec.Client.SecretName), - buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial "%s" for spec.githubAPI.host (%q): dial tcp: lookup nowhere.bad-tld: no such host`, "nowhere.bad-tld:443", "nowhere.bad-tld")), + buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot make connection probe request for spec.githubAPI.host (%q): dial tcp: lookup nowhere.bad-tld: no such host`, "nowhere.bad-tld")), buildHostValidTrue(t, "nowhere.bad-tld"), buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), buildTLSConfigurationValidTrue(t), @@ -1791,7 +1756,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "nowhere.bad-tld"), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "False", "UnableToDialServer", `cannot dial \"%s\" for spec.githubAPI.host (\"%s\"): dial tcp: lookup nowhere.bad-tld: no such host`, "nowhere.bad-tld:443", "nowhere.bad-tld"), + buildLogForUpdatingGitHubConnectionInvalid("minimal-idp-name", fmt.Sprintf(`cannot make connection probe request for spec.githubAPI.host (\"%s\"): dial tcp: lookup nowhere.bad-tld: no such host`, "nowhere.bad-tld")), buildLogForUpdatingPhase("minimal-idp-name", "Error"), }, }, @@ -1860,7 +1825,7 @@ func TestController(t *testing.T) { Conditions: []metav1.Condition{ buildClaimsValidatedTrue(t), buildClientCredentialsSecretValidTrue(t, validFilledOutIDP.Spec.Client.SecretName), - buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial "%s" for spec.githubAPI.host (%q): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host)), + buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot make connection probe request for spec.githubAPI.host (%q): Get %q: tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, fmt.Sprintf("https://%s/api/v3/rate_limit", *validFilledOutIDP.Spec.GitHubAPI.Host))), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), buildTLSConfigurationValidTrueWithMsg(t, "no TLS configuration provided: using default root CA bundle from container image"), @@ -1874,7 +1839,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: no TLS configuration provided: using default root CA bundle from container image"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "False", "UnableToDialServer", `cannot dial \"%s\" for spec.githubAPI.host (\"%s\"): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionInvalid("some-idp-name", fmt.Sprintf(`cannot make connection probe request for spec.githubAPI.host (\"%s\"): Get \"%s\": tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, fmt.Sprintf("https://%s/api/v3/rate_limit", *validFilledOutIDP.Spec.GitHubAPI.Host))), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -1906,7 +1871,7 @@ func TestController(t *testing.T) { Conditions: []metav1.Condition{ buildClaimsValidatedTrue(t), buildClientCredentialsSecretValidTrue(t, validFilledOutIDP.Spec.Client.SecretName), - buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial "%s" for spec.githubAPI.host (%q): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host)), + buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot make connection probe request for spec.githubAPI.host (%q): Get %q: tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, fmt.Sprintf("https://%s/api/v3/rate_limit", *validFilledOutIDP.Spec.GitHubAPI.Host))), buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), buildTLSConfigurationValidTrue(t), @@ -1920,7 +1885,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "False", "UnableToDialServer", `cannot dial \"%s\" for spec.githubAPI.host (\"%s\"): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionInvalid("some-idp-name", fmt.Sprintf(`cannot make connection probe request for spec.githubAPI.host (\"%s\"): Get \"%s\": tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host, fmt.Sprintf("https://%s/api/v3/rate_limit", *validFilledOutIDP.Spec.GitHubAPI.Host))), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -1961,7 +1926,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2002,7 +1967,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2043,7 +2008,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2084,7 +2049,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty"), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2125,7 +2090,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2166,7 +2131,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2207,7 +2172,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2248,7 +2213,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("some-idp-name", "Error"), }, }, @@ -2291,7 +2256,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("minimal-idp-name", "Error"), }, }, @@ -2334,7 +2299,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("minimal-idp-name", "Error"), }, }, @@ -2377,7 +2342,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("minimal-idp-name", "Error"), }, }, @@ -2420,7 +2385,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host, *validMinimalIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("minimal-idp-name", "Error"), }, }, @@ -2463,7 +2428,7 @@ func TestController(t *testing.T) { buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls is valid: using configured CA bundle"), - buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `dialed \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `probed connection to \"%s\" for spec.githubAPI.host (\"%s\"): host is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host, *validFilledOutIDP.Spec.GitHubAPI.Host), buildLogForUpdatingPhase("minimal-idp-name", "Error"), }, }, @@ -2491,9 +2456,9 @@ func TestController(t *testing.T) { gitHubIdentityProviderInformer := supervisorInformers.IDP().V1alpha1().GitHubIdentityProviders() - var dialer ptls.Dialer = ptls.NewDialer() - if tt.mockDialer != nil { - dialer = tt.mockDialer(t) + var probeURLFunc ProbeURLFunc = ProbeURL // by default use the production code for probing a URL + if tt.mockProbeURLFunc != nil { + probeURLFunc = tt.mockProbeURLFunc(t) } validatedCache := cache.NewExpiring() @@ -2511,7 +2476,7 @@ func TestController(t *testing.T) { logger, controllerlib.WithInformer, frozenClockForLastTransitionTime, - dialer, + probeURLFunc, validatedCache, ) @@ -2722,7 +2687,7 @@ func TestController_OnlyWantActions(t *testing.T) { ObservedGeneration: 333, LastTransitionTime: oneHourAgo, Reason: conditionsutil.ReasonSuccess, - Message: fmt.Sprintf("dialed %q for spec.githubAPI.host (%q): host is reachable and TLS verification succeeds", goodServerDomain, goodServerDomain), + Message: fmt.Sprintf("probed connection to %q for spec.githubAPI.host (%q): host is reachable and TLS verification succeeds", goodServerDomain, goodServerDomain), }, { Type: HostValid, @@ -2834,7 +2799,7 @@ func TestController_OnlyWantActions(t *testing.T) { ObservedGeneration: 1234, LastTransitionTime: wantLastTransitionTime, Reason: conditionsutil.ReasonSuccess, - Message: fmt.Sprintf("dialed %q for spec.githubAPI.host (%q): host is reachable and TLS verification succeeds", goodServerDomain, goodServerDomain), + Message: fmt.Sprintf("probed connection to %q for spec.githubAPI.host (%q): host is reachable and TLS verification succeeds", goodServerDomain, goodServerDomain), }, { Type: HostValid, @@ -2894,7 +2859,7 @@ func TestController_OnlyWantActions(t *testing.T) { logger, controllerlib.WithInformer, frozenClockForLastTransitionTime, - ptls.NewDialer(), + ProbeURL, cache.NewExpiring(), ) @@ -3018,7 +2983,7 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { logger, observableInformers.WithInformer, clock.RealClock{}, - ptls.NewDialer(), + nil, cache.NewExpiring(), ) @@ -3075,7 +3040,7 @@ func TestGitHubUpstreamWatcherControllerFilterConfigMaps(t *testing.T) { logger, observableInformers.WithInformer, clock.RealClock{}, - ptls.NewDialer(), + nil, cache.NewExpiring(), ) @@ -3132,7 +3097,7 @@ func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { logger, observableInformers.WithInformer, clock.RealClock{}, - ptls.NewDialer(), + nil, cache.NewExpiring(), ) diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 0dfc2396a..b3aa5468c 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -36,6 +36,7 @@ import ( "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/leaderelection" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/proxydetect" "go.pinniped.dev/internal/tokenclient" ) @@ -246,6 +247,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol clock.RealClock{}, plog.New(), ptls.NewDialer(), + proxydetect.New(), ), singletonWorker, ). diff --git a/internal/proxydetect/proxydetect.go b/internal/proxydetect/proxydetect.go new file mode 100644 index 000000000..b7b8c892c --- /dev/null +++ b/internal/proxydetect/proxydetect.go @@ -0,0 +1,64 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package proxydetect + +import ( + "context" + "fmt" + "net/http" + "net/url" + + "k8s.io/apimachinery/pkg/util/net" +) + +type ProxyDetect interface { + // UsingProxyForHost returns true if HTTPS requests made to the specified host would be sent through a web proxy. + // It returns false if requests would not be sent through a proxy. It returns an error if it cannot be determined. + UsingProxyForHost(host string) (bool, error) +} + +type detector struct { + // The real http.ProxyFromEnvironment func only reads the env vars once, and then never reads them again + // for the rest of the process's lifetime. This makes it hard to write unit tests that use the real func, + // because you cannot vary the env variables' values between tests, so we'll use a fake in unit tests. + proxyFromEnvironmentFunc func(req *http.Request) (*url.URL, error) +} + +var _ ProxyDetect = (*detector)(nil) + +func New() ProxyDetect { + return &detector{ + // Because this is intended for use with the WebhookAuthenticator, let's + // use the same proxy function that we use for the WebhookAuthenticator. + // Refer to how webhookcachefiller.go constructs the webhook authenticator, + // and you'll find how it sets the proxy function in kubeclient.go to be this... + proxyFromEnvironmentFunc: net.NewProxierWithNoProxyCIDR(http.ProxyFromEnvironment), + } +} + +func (d *detector) UsingProxyForHost(host string) (bool, error) { + const msgFmt = "could not determine if requests will be proxied for host %q: %v" + + // Make a request object that represents any HTTPS request to the specified server. + // The other parameter values don't matter, as long as they are valid, because we won't actually make this request. + r, err := http.NewRequestWithContext( + context.Background(), + http.MethodGet, + fmt.Sprintf("https://%s", host), + nil, + ) + if err != nil { + // This will return an error if the host string has an invalid format. + return false, fmt.Errorf(msgFmt, host, err) + } + + // Ask if the request would use a proxy or not. This does not actually make the request. + proxyURL, err := d.proxyFromEnvironmentFunc(r) + if err != nil { + // This could return an error if the HTTPS_PROXY env variable's value had an invalid format, for example. + return false, fmt.Errorf(msgFmt, host, err) + } + + return proxyURL != nil, nil +} diff --git a/internal/proxydetect/proxydetect_test.go b/internal/proxydetect/proxydetect_test.go new file mode 100644 index 000000000..950b5c6f1 --- /dev/null +++ b/internal/proxydetect/proxydetect_test.go @@ -0,0 +1,168 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package proxydetect + +import ( + "errors" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestProxyDetectWithoutMock(t *testing.T) { + // Setting these real env vars means that we cannot run this test in parallel. + t.Setenv("HTTPS_PROXY", "http://proxy.pinniped.dev") + t.Setenv("NO_PROXY", "1.2.3.4,1.2.3.5:3333,4.4.4.0/28,example1.pinniped.dev:8443,example2.pinniped.dev") + + subject := New() + + tests := []struct { + name string + host string + wantProxied bool + wantErr string + }{ + // This does not test all permutations of how HTTPS_PROXY and NO_PROXY work. + // Some basic tests to understand how these settings work are included below. + // See https://pkg.go.dev/golang.org/x/net/http/httpproxy for docs. + { + name: "any host not included in NO_PROXY should use the proxy", + host: "www.pinniped.dev", + wantProxied: true, + }, + { + name: "a port different from the one specified in NO_PROXY should use the proxy, for the default HTTPS port", + host: "example1.pinniped.dev", + wantProxied: true, + }, + { + name: "a port different from the one specified in NO_PROXY should use the proxy, for an explicit port", + host: "example1.pinniped.dev:994", + wantProxied: true, + }, + { + name: "same port as the one specified in NO_PROXY should skip the proxy", + host: "example1.pinniped.dev:8443", + wantProxied: false, + }, + { + name: "any host included in NO_PROXY should skip the proxy, with default ports", + host: "example2.pinniped.dev", + wantProxied: false, + }, + { + name: "an IP specified in NO_PROXY should skip the proxy", + host: "1.2.3.4", + wantProxied: false, + }, + { + name: "an IP specified in NO_PROXY should skip the proxy, with matching explicit ports", + host: "1.2.3.5:3333", + wantProxied: false, + }, + { + name: "an IP specified in NO_PROXY should use the proxy when the ports don't match", + host: "1.2.3.5:1234", + wantProxied: true, + }, + { + name: "an IP included in a NO_PROXY CIDR should skip the proxy", + host: "4.4.4.4", + wantProxied: false, + }, + { + name: "an IP outside a NO_PROXY CIDR should use the proxy", + host: "4.4.4.16", + wantProxied: true, + }, + { + name: "as a special case in the Go documentation, localhost never uses the proxy, regardless of NO_PROXY settings", + host: "localhost", + wantProxied: false, + }, + { + name: "a bad hostname returns an error", + host: "bad hostname", + wantProxied: false, + wantErr: `could not determine if requests will be proxied for host "bad hostname": parse "https://bad hostname": invalid character " " in host name`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + usingProxyForHost, err := subject.UsingProxyForHost(tt.host) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantProxied, usingProxyForHost) + }) + } +} + +func TestProxyDetectWithMock(t *testing.T) { + proxyURL, err := url.Parse("http://myproxy.com") + require.NoError(t, err) + + tests := []struct { + name string + + proxyFromEnvironmentReturnsURL *url.URL + proxyFromEnvironmentReturnsErr error + + host string + + wantProxied bool + wantErr string + }{ + { + name: "when using proxy for host", + proxyFromEnvironmentReturnsURL: proxyURL, + host: "example.com", + wantProxied: true, + }, + { + name: "when not using proxy for host", + proxyFromEnvironmentReturnsURL: nil, + host: "example.com", + wantProxied: false, + }, + { + name: "when ProxyFromEnvironment returns an error", + proxyFromEnvironmentReturnsErr: errors.New("some error"), + host: "example.com", + wantProxied: false, + wantErr: `could not determine if requests will be proxied for host "example.com": some error`, + }, + { + name: "invalid host", + host: "invalid hostname", + wantProxied: false, + wantErr: `could not determine if requests will be proxied for host "invalid hostname": parse "https://invalid hostname": invalid character " " in host name`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + subject := detector{proxyFromEnvironmentFunc: func(req *http.Request) (*url.URL, error) { + return tt.proxyFromEnvironmentReturnsURL, tt.proxyFromEnvironmentReturnsErr + }} + + proxied, err := subject.UsingProxyForHost(tt.host) + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantProxied, proxied) + }) + } +} diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index ae2b65167..83c28664a 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -342,7 +342,7 @@ func prepareControllers( plog.New(), controllerlib.WithInformer, clock.RealClock{}, - ptls.NewDialer(), + githubupstreamwatcher.ProbeURL, cache.NewExpiring(), ), singletonWorker). diff --git a/internal/testutil/fakeproxydetect/fakeproxydetect.go b/internal/testutil/fakeproxydetect/fakeproxydetect.go new file mode 100644 index 000000000..5d45e47db --- /dev/null +++ b/internal/testutil/fakeproxydetect/fakeproxydetect.go @@ -0,0 +1,36 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package fakeproxydetect + +import "go.pinniped.dev/internal/proxydetect" + +type FakeProxyDetect struct { + returnBool bool + returnErr error + numCalls int + receivedHost string +} + +var _ proxydetect.ProxyDetect = (*FakeProxyDetect)(nil) + +func (f *FakeProxyDetect) UsingProxyForHost(host string) (bool, error) { + f.numCalls++ + f.receivedHost = host + return f.returnBool, f.returnErr +} + +func (f *FakeProxyDetect) ReceivedHostDuringMostRecentInvocation() string { + return f.receivedHost +} + +func (f *FakeProxyDetect) NumberOfInvocations() int { + return f.numCalls +} + +func New(returnBool bool, returnErr error) *FakeProxyDetect { + return &FakeProxyDetect{ + returnBool: returnBool, + returnErr: returnErr, + } +} diff --git a/test/integration/concierge_jwtauthenticator_status_test.go b/test/integration/concierge_jwtauthenticator_status_test.go index bc2d502b9..2a34d5816 100644 --- a/test/integration/concierge_jwtauthenticator_status_test.go +++ b/test/integration/concierge_jwtauthenticator_status_test.go @@ -192,7 +192,7 @@ func TestConciergeJWTAuthenticatorStatus_Parallel(t *testing.T) { }, }, wantPhase: authenticationv1alpha1.JWTAuthenticatorPhaseError, - wantConditions: replaceSomeConditions( + wantConditions: replaceSomeConditions(t, allSuccessfulJWTAuthenticatorConditions(true), []metav1.Condition{ { @@ -242,7 +242,7 @@ func TestConciergeJWTAuthenticatorStatus_Parallel(t *testing.T) { }, }, wantPhase: authenticationv1alpha1.JWTAuthenticatorPhaseError, - wantConditions: replaceSomeConditions( + wantConditions: replaceSomeConditions(t, allSuccessfulJWTAuthenticatorConditions(true), []metav1.Condition{ { @@ -286,7 +286,7 @@ func TestConciergeJWTAuthenticatorStatus_Parallel(t *testing.T) { }, }, wantPhase: authenticationv1alpha1.JWTAuthenticatorPhaseError, - wantConditions: replaceSomeConditions( + wantConditions: replaceSomeConditions(t, allSuccessfulJWTAuthenticatorConditions(len(env.SupervisorUpstreamOIDC.CABundle) != 0), []metav1.Condition{ { diff --git a/test/integration/concierge_webhookauthenticator_status_test.go b/test/integration/concierge_webhookauthenticator_status_test.go index c5a9dec3c..1444ac204 100644 --- a/test/integration/concierge_webhookauthenticator_status_test.go +++ b/test/integration/concierge_webhookauthenticator_status_test.go @@ -153,22 +153,24 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) t.Cleanup(cancel) + conciergeUsingHTTPSProxy := testlib.DeploymentsContainerHasHTTPSProxyEnvVar(t, env.ConciergeNamespace, env.ConciergeAppName) + caBundleSomePivotalCA := "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVVENDQWptZ0F3SUJBZ0lWQUpzNStTbVRtaTJXeUI0bGJJRXBXaUs5a1RkUE1BMEdDU3FHU0liM0RRRUIKQ3dVQU1COHhDekFKQmdOVkJBWVRBbFZUTVJBd0RnWURWUVFLREFkUWFYWnZkR0ZzTUI0WERUSXdNRFV3TkRFMgpNamMxT0ZvWERUSTBNRFV3TlRFMk1qYzFPRm93SHpFTE1Ba0dBMVVFQmhNQ1ZWTXhFREFPQmdOVkJBb01CMUJwCmRtOTBZV3d3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRRERZWmZvWGR4Z2NXTEMKZEJtbHB5a0tBaG9JMlBuUWtsVFNXMno1cGcwaXJjOGFRL1E3MXZzMTRZYStmdWtFTGlvOTRZYWw4R01DdVFrbApMZ3AvUEE5N1VYelhQNDBpK25iNXcwRGpwWWd2dU9KQXJXMno2MFRnWE5NSFh3VHk4ME1SZEhpUFVWZ0VZd0JpCmtkNThzdEFVS1Y1MnBQTU1reTJjNy9BcFhJNmRXR2xjalUvaFBsNmtpRzZ5dEw2REtGYjJQRWV3MmdJM3pHZ2IKOFVVbnA1V05DZDd2WjNVY0ZHNXlsZEd3aGc3cnZ4U1ZLWi9WOEhCMGJmbjlxamlrSVcxWFM4dzdpUUNlQmdQMApYZWhKZmVITlZJaTJtZlczNlVQbWpMdnVKaGpqNDIrdFBQWndvdDkzdWtlcEgvbWpHcFJEVm9wamJyWGlpTUYrCkYxdnlPNGMxQWdNQkFBR2pnWU13Z1lBd0hRWURWUjBPQkJZRUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1IKTUI4R0ExVWRJd1FZTUJhQUZNTWJpSXFhdVkwajRVWWphWDl0bDJzby9LQ1JNQjBHQTFVZEpRUVdNQlFHQ0NzRwpBUVVGQndNQ0JnZ3JCZ0VGQlFjREFUQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BNEdBMVVkRHdFQi93UUVBd0lCCkJqQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFYbEh4M2tIMDZwY2NDTDlEVE5qTnBCYnlVSytGd2R6T2IwWFYKcmpNaGtxdHVmdEpUUnR5T3hKZ0ZKNXhUR3pCdEtKamcrVU1pczBOV0t0VDBNWThVMU45U2c5SDl0RFpHRHBjVQpxMlVRU0Y4dXRQMVR3dnJIUzIrdzB2MUoxdHgrTEFiU0lmWmJCV0xXQ21EODUzRlVoWlFZekkvYXpFM28vd0p1CmlPUklMdUpNUk5vNlBXY3VLZmRFVkhaS1RTWnk3a25FcHNidGtsN3EwRE91eUFWdG9HVnlkb3VUR0FOdFhXK2YKczNUSTJjKzErZXg3L2RZOEJGQTFzNWFUOG5vZnU3T1RTTzdiS1kzSkRBUHZOeFQzKzVZUXJwNGR1Nmh0YUFMbAppOHNaRkhidmxpd2EzdlhxL3p1Y2JEaHEzQzBhZnAzV2ZwRGxwSlpvLy9QUUFKaTZLQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K" tests := []struct { name string + maybeSkip func(t *testing.T) spec func() *authenticationv1alpha1.WebhookAuthenticatorSpec initialPhase authenticationv1alpha1.WebhookAuthenticatorPhase finalConditions []metav1.Condition - run func(t *testing.T) }{ { - name: "basic test to see if the WebhookAuthenticator wakes up or not", + name: "happy path", spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { return &env.TestWebhook }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseReady, - finalConditions: allSuccessfulWebhookAuthenticatorConditions(), + finalConditions: allSuccessfulWebhookAuthenticatorConditions(conciergeUsingHTTPSProxy), }, { name: "valid spec with invalid CA in TLS config will result in a WebhookAuthenticator that is not ready", @@ -181,8 +183,8 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { return webhookSpec }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, - finalConditions: replaceSomeConditions( - allSuccessfulWebhookAuthenticatorConditions(), + finalConditions: replaceSomeConditions(t, + allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { Type: "Ready", @@ -210,6 +212,14 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { }, { name: "valid spec with valid CA in TLS config but does not match issuer server will result in a WebhookAuthenticator that is not ready", + maybeSkip: func(t *testing.T) { + if conciergeUsingHTTPSProxy { + // Skip this test when HTTPS_PROXY is in use, because WebhookConnectionValid will have status Success + // with a message saying that the dialing was skipped due to the proxy setting, so the expectations + // below are wrong for that case. + t.Skip("Skipping test that requires HTTPS_PROXY to be unset") + } + }, spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { webhookSpec := env.TestWebhook.DeepCopy() webhookSpec.TLS = &authenticationv1alpha1.TLSSpec{ @@ -218,8 +228,8 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { return webhookSpec }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, - finalConditions: replaceSomeConditions( - allSuccessfulWebhookAuthenticatorConditions(), + finalConditions: replaceSomeConditions(t, + allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { Type: "Ready", @@ -247,12 +257,16 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { webhookSpec.TLS = &authenticationv1alpha1.TLSSpec{ CertificateAuthorityData: caBundleSomePivotalCA, } - webhookSpec.Endpoint = "https://127.0.0.1:443/some-fake-endpoint" + // Note that requests to 127.0.0.1 will not be proxied even when HTTPS_PROXY is set on the Concierge, + // so it's okay to run this test in that case too. The Concierge will attempt the dial because it sees + // that the request would not be proxied anyway. 127.0.0.1:8781 will be seen by the Concierge as a port + // local to its own pod, which does not exist so the connection will fail. + webhookSpec.Endpoint = "https://127.0.0.1:8781/some-fake-endpoint" return webhookSpec }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, - finalConditions: replaceSomeConditions( - allSuccessfulWebhookAuthenticatorConditions(), + finalConditions: replaceSomeConditions(t, + allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { Type: "Ready", @@ -268,7 +282,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { Type: "WebhookConnectionValid", Status: "False", Reason: "UnableToDialServer", - Message: "cannot dial server: dial tcp 127.0.0.1:443: connect: connection refused", + Message: "cannot dial server: dial tcp 127.0.0.1:8781: connect: connection refused", }, }, ), @@ -279,6 +293,10 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() + if tt.maybeSkip != nil { + tt.maybeSkip(t) + } + webhookAuthenticator := testlib.CreateTestWebhookAuthenticator( ctx, t, @@ -397,7 +415,12 @@ func TestConciergeWebhookAuthenticatorCRDValidations_Parallel(t *testing.T) { } } -func allSuccessfulWebhookAuthenticatorConditions() []metav1.Condition { +func allSuccessfulWebhookAuthenticatorConditions(conciergeUsingHTTPSProxy bool) []metav1.Condition { + dialMessage := "successfully dialed webhook server" + if conciergeUsingHTTPSProxy { + dialMessage = "skipped dialing connection probe because HTTPS_PROXY is configured for use with the specified host" + } + return []metav1.Condition{ { Type: "AuthenticatorValid", @@ -427,7 +450,7 @@ func allSuccessfulWebhookAuthenticatorConditions() []metav1.Condition { Type: "WebhookConnectionValid", Status: "True", Reason: "Success", - Message: "successfully dialed webhook server", + Message: dialMessage, }, } } diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index dc6897df7..2d54abcd3 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -2196,7 +2196,8 @@ func requireUserCanUseKubectlWithoutAuthenticatingAgain( func requireGCAnnotationsOnSessionStorage(ctx context.Context, t *testing.T, supervisorNamespace string, startTime time.Time, token *oidctypes.Token) { // check that the access token is new (since it's just been refreshed) and has close to two minutes left. - testutil.RequireTimeInDelta(t, startTime.Add(2*time.Minute), token.AccessToken.Expiry.Time, 15*time.Second) + // Give some significant fudge factor to allow for slow CI workers. + testutil.RequireTimeInDelta(t, startTime.Add(2*time.Minute), token.AccessToken.Expiry.Time, 35*time.Second) kubeClient := testlib.NewKubernetesClientset(t).CoreV1() diff --git a/test/integration/supervisor_federationdomain_status_test.go b/test/integration/supervisor_federationdomain_status_test.go index 27877b616..712bb6577 100644 --- a/test/integration/supervisor_federationdomain_status_test.go +++ b/test/integration/supervisor_federationdomain_status_test.go @@ -46,8 +46,8 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { fd := testlib.CreateTestFederationDomain(ctx, t, supervisorconfigv1alpha1.FederationDomainSpec{ Issuer: "https://example.com/fake", }, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( - allSuccessfulLegacyFederationDomainConditions("", fd.Spec), + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, + allSuccessfulLegacyFederationDomainConditions(t, "", fd.Spec), []metav1.Condition{ { Type: "IdentityProvidersFound", Status: "False", Reason: "LegacyConfigurationIdentityProviderNotFound", @@ -67,7 +67,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }, idpv1alpha1.PhaseError) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady) testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, - allSuccessfulLegacyFederationDomainConditions(oidcIdentityProvider1.Name, fd.Spec)) + allSuccessfulLegacyFederationDomainConditions(t, oidcIdentityProvider1.Name, fd.Spec)) // Creating a second IDP should put the FederationDomain back into an error status again. oidcIdentityProvider2 := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ @@ -75,8 +75,8 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, }, idpv1alpha1.PhaseError) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( - allSuccessfulLegacyFederationDomainConditions(oidcIdentityProvider2.Name, fd.Spec), + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, + allSuccessfulLegacyFederationDomainConditions(t, oidcIdentityProvider2.Name, fd.Spec), []metav1.Condition{ { Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProviderNotSpecified", @@ -121,7 +121,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }, }, }, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -145,7 +145,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, }, oidcIDP1Meta, idpv1alpha1.PhaseError) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -173,7 +173,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { err := oidcIDPClient.Delete(ctx, oidcIdentityProvider1.Name, metav1.DeleteOptions{}) require.NoError(t, err) testlib.WaitForFederationDomainStatusPhase(ctx, t, fd.Name, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -340,7 +340,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }, }, supervisorconfigv1alpha1.FederationDomainPhaseError) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -483,7 +483,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { }) require.NoError(t, err) - testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions( + testlib.WaitForFederationDomainStatusConditions(ctx, t, fd.Name, replaceSomeConditions(t, allSuccessfulFederationDomainConditions(fd.Spec), []metav1.Condition{ { @@ -950,22 +950,30 @@ func TestSupervisorFederationDomainCRDValidations_Parallel(t *testing.T) { } } -func replaceSomeConditions(conditions []metav1.Condition, replaceWithTheseConditions []metav1.Condition) []metav1.Condition { +func replaceSomeConditions(t *testing.T, conditions []metav1.Condition, replaceWithTheseConditions []metav1.Condition) []metav1.Condition { cp := make([]metav1.Condition, len(conditions)) copy(cp, conditions) for _, replacementCond := range replaceWithTheseConditions { + found := false for i, cond := range cp { if replacementCond.Type == cond.Type { cp[i] = replacementCond + found = true break } } + if !found { + require.Failf(t, + "test setup problem", + "replaceSomeConditions() helper was called to replace condition of type %q but no such condition was found in conditions slice", + replacementCond.Type) + } } return cp } -func allSuccessfulLegacyFederationDomainConditions(idpName string, federationDomainSpec supervisorconfigv1alpha1.FederationDomainSpec) []metav1.Condition { - return replaceSomeConditions( +func allSuccessfulLegacyFederationDomainConditions(t *testing.T, idpName string, federationDomainSpec supervisorconfigv1alpha1.FederationDomainSpec) []metav1.Condition { + return replaceSomeConditions(t, allSuccessfulFederationDomainConditions(federationDomainSpec), []metav1.Condition{ { diff --git a/test/integration/supervisor_github_idp_test.go b/test/integration/supervisor_github_idp_test.go index bd713254a..ae71060d4 100644 --- a/test/integration/supervisor_github_idp_test.go +++ b/test/integration/supervisor_github_idp_test.go @@ -392,7 +392,7 @@ func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { Type: "GitHubConnectionValid", Status: metav1.ConditionTrue, Reason: "Success", - Message: `dialed "api.github.com:443" for spec.githubAPI.host ("github.com"): host is reachable and TLS verification succeeds`, + Message: `probed connection to "api.github.com:443" for spec.githubAPI.host ("github.com"): host is reachable and TLS verification succeeds`, }, { Type: "HostValid", @@ -461,7 +461,7 @@ func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { Type: "GitHubConnectionValid", Status: metav1.ConditionTrue, Reason: "Success", - Message: `dialed "api.github.com:443" for spec.githubAPI.host ("api.github.com"): host is reachable and TLS verification succeeds`, + Message: `probed connection to "api.github.com:443" for spec.githubAPI.host ("api.github.com"): host is reachable and TLS verification succeeds`, }, { Type: "HostValid", @@ -668,7 +668,7 @@ func TestGitHubIDPSecretInOtherNamespace_Parallel(t *testing.T) { Type: "GitHubConnectionValid", Status: metav1.ConditionTrue, Reason: "Success", - Message: `dialed "api.github.com:443" for spec.githubAPI.host ("github.com"): host is reachable and TLS verification succeeds`, + Message: `probed connection to "api.github.com:443" for spec.githubAPI.host ("github.com"): host is reachable and TLS verification succeeds`, }, { Type: "HostValid", diff --git a/test/testlib/client.go b/test/testlib/client.go index 731e26d21..01fa87b4f 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -1109,3 +1109,23 @@ func ObjectMetaWithRandomName(t *testing.T, baseName string) metav1.ObjectMeta { Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, } } + +func DeploymentsContainerHasHTTPSProxyEnvVar(t *testing.T, namespaceName string, deploymentName string) bool { + client := NewKubernetesClientset(t) + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + d, err := client.AppsV1().Deployments(namespaceName).Get(ctx, deploymentName, metav1.GetOptions{}) + require.NoError(t, err) + + for _, c := range d.Spec.Template.Spec.Containers { + for _, e := range c.Env { + if e.Name == "HTTPS_PROXY" { + return true + } + } + } + + return false +}