From 4f661aaa693320e885c11721a22b8cd289180887 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 9 Oct 2024 17:11:00 -0700 Subject: [PATCH] pay attention to web proxy settings during connection probes - WebhookAuthenticator will now detect the proxy setting and skip dialing the connection probe if it should go through a proxy - GitHubIdentityProvider will avoid using tls.Dial altogether by instead making a real request to the GitHub API as its connection probe, because this will respect the proxy settings --- .../webhookcachefiller/webhookcachefiller.go | 67 ++++-- .../webhookcachefiller_test.go | 227 +++++++++++++++++- .../conditionsutil/conditions_util.go | 9 +- .../github_upstream_watcher.go | 89 +++++-- .../github_upstream_watcher_test.go | 225 +++++++---------- .../controllermanager/prepare_controllers.go | 2 + internal/proxydetect/proxydetect.go | 56 +++++ internal/proxydetect/proxydetect_test.go | 77 ++++++ internal/supervisor/server/server.go | 2 +- .../fakeproxydetect/fakeproxydetect.go | 36 +++ ...cierge_webhookauthenticator_status_test.go | 36 ++- .../integration/supervisor_github_idp_test.go | 6 +- test/testlib/client.go | 20 ++ 13 files changed, 643 insertions(+), 209 deletions(-) create mode 100644 internal/proxydetect/proxydetect.go create mode 100644 internal/proxydetect/proxydetect_test.go create mode 100644 internal/testutil/fakeproxydetect/fakeproxydetect.go diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index edd11b423..15a1bed38 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -39,6 +39,7 @@ import ( "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/proxydetect" ) const ( @@ -49,10 +50,12 @@ const ( typeEndpointURLValid = "EndpointURLValid" typeAuthenticatorValid = "AuthenticatorValid" - reasonUnableToCreateClient = "UnableToCreateClient" - reasonUnableToInstantiateWebhook = "UnableToInstantiateWebhook" - reasonInvalidEndpointURL = "InvalidEndpointURL" - reasonInvalidEndpointURLScheme = "InvalidEndpointURLScheme" + reasonUnableToCreateClient = "UnableToCreateClient" + reasonUnableToInstantiateWebhook = "UnableToInstantiateWebhook" + reasonInvalidEndpointURL = "InvalidEndpointURL" + reasonInvalidEndpointURLScheme = "InvalidEndpointURLScheme" + reasonInvalidEndpointCannotDetermineProxy = "InvalidEndpointCannotDetermineProxy" + reasonUnableToDialServer = "UnableToDialServer" ) type cachedWebhookAuthenticator struct { @@ -77,6 +80,7 @@ func New( clock clock.Clock, log plog.Logger, dialer ptls.Dialer, + proxyDetector proxydetect.ProxyDetect, ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ @@ -91,6 +95,7 @@ func New( clock: clock, log: log.WithName(controllerName), dialer: dialer, + proxyDetector: proxyDetector, }, }, withInformer( @@ -127,6 +132,7 @@ type webhookCacheFillerController struct { clock clock.Clock log plog.Logger dialer ptls.Dialer + proxyDetector proxydetect.ProxyDetect } // Sync implements controllerlib.Syncer. @@ -173,7 +179,7 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co caBundle, conditions, tlsBundleOk := c.validateTLSBundle(webhookAuthenticator.Spec.TLS, conditions) - endpointHostPort, conditions, endpointOk := c.validateEndpoint(webhookAuthenticator.Spec.Endpoint, conditions) + endpointHostPort, conditions, usingProxyForHost, endpointOk := c.validateEndpoint(webhookAuthenticator.Spec.Endpoint, conditions) okSoFar := tlsBundleOk && endpointOk // Only revalidate and update the cache if the cached authenticator is different from the desired authenticator. @@ -194,12 +200,14 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co // if they need to be updated. logger.Info("cached webhook authenticator and desired webhook authenticator are the same: already cached, so skipping validations") conditions = append(conditions, - successfulWebhookConnectionValidCondition(), + successfulWebhookConnectionValidCondition(usingProxyForHost), successfulAuthenticatorValidCondition(), ) } else { // Run all remaining validations. - a, moreConditions, moreErrs := c.doExpensiveValidations(ctx, webhookAuthenticator, endpointHostPort, caBundle, okSoFar, logger) + a, moreConditions, moreErrs := c.doExpensiveValidations( + ctx, webhookAuthenticator, endpointHostPort, caBundle, okSoFar, usingProxyForHost, logger, + ) newWebhookAuthenticatorForCache = a conditions = append(conditions, moreConditions...) errs = append(errs, moreErrs...) @@ -243,13 +251,14 @@ func (c *webhookCacheFillerController) doExpensiveValidations( endpointHostPort *endpointaddr.HostPort, caBundle *tlsconfigutil.CABundle, okSoFar bool, + usingProxyForHost bool, logger plog.Logger, ) (*cachedWebhookAuthenticator, []*metav1.Condition, []error) { var newWebhookAuthenticatorForCache *cachedWebhookAuthenticator var conditions []*metav1.Condition var errs []error - conditions, tlsNegotiateErr := c.validateConnection(ctx, caBundle.CertPool(), endpointHostPort, conditions, okSoFar, logger) + conditions, tlsNegotiateErr := c.validateConnection(ctx, caBundle.CertPool(), endpointHostPort, conditions, okSoFar, usingProxyForHost, logger) errs = append(errs, tlsNegotiateErr) okSoFar = okSoFar && tlsNegotiateErr == nil @@ -405,12 +414,16 @@ func newWebhookAuthenticator( return webhookAuthenticator, conditions, nil } -func successfulWebhookConnectionValidCondition() *metav1.Condition { +func successfulWebhookConnectionValidCondition(usingProxyForHost bool) *metav1.Condition { + msg := "successfully dialed webhook server" + if usingProxyForHost { + msg = "skipped dialing connection probe because HTTPS_PROXY is configured for use with the specified host" + } return &metav1.Condition{ Type: typeWebhookConnectionValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, - Message: "successfully dialed webhook server", + Message: msg, } } @@ -420,6 +433,7 @@ func (c *webhookCacheFillerController) validateConnection( endpointHostPort *endpointaddr.HostPort, conditions []*metav1.Condition, prereqOk bool, + usingProxyForHost bool, logger plog.Logger, ) ([]*metav1.Condition, error) { if !prereqOk { @@ -432,6 +446,13 @@ func (c *webhookCacheFillerController) validateConnection( return conditions, nil } + if usingProxyForHost { + // We cannot assume that we can directly dial the host in the case where we should + // be using a web proxy to reach the host, so skip the dial probe in that case. + conditions = append(conditions, successfulWebhookConnectionValidCondition(usingProxyForHost)) + return conditions, nil + } + dialCtx, dialCancel := context.WithTimeout(ctx, 30*time.Second) defer dialCancel() err := c.dialer.IsReachableAndTLSValidationSucceeds(dialCtx, endpointHostPort.Endpoint(), certPool, logger) @@ -442,17 +463,17 @@ func (c *webhookCacheFillerController) validateConnection( conditions = append(conditions, &metav1.Condition{ Type: typeWebhookConnectionValid, Status: metav1.ConditionFalse, - Reason: conditionsutil.ReasonUnableToDialServer, + Reason: reasonUnableToDialServer, Message: msg, }) return conditions, fmt.Errorf("%s: %w", errText, err) } - conditions = append(conditions, successfulWebhookConnectionValidCondition()) + conditions = append(conditions, successfulWebhookConnectionValidCondition(usingProxyForHost)) return conditions, nil } -func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditions []*metav1.Condition) (*endpointaddr.HostPort, []*metav1.Condition, bool) { +func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditions []*metav1.Condition) (*endpointaddr.HostPort, []*metav1.Condition, bool, bool) { endpointURL, err := url.Parse(endpoint) if err != nil { msg := fmt.Sprintf("%s: %s", "spec.endpoint URL cannot be parsed", err.Error()) @@ -462,7 +483,7 @@ func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditi Reason: reasonInvalidEndpointURL, Message: msg, }) - return nil, conditions, false + return nil, conditions, false, false } // handles empty string and other issues as well. @@ -474,7 +495,7 @@ func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditi Reason: reasonInvalidEndpointURLScheme, Message: msg, }) - return nil, conditions, false + return nil, conditions, false, false } endpointHostPort, err := endpointaddr.ParseFromURL(endpointURL, 443) @@ -486,7 +507,19 @@ func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditi Reason: reasonInvalidEndpointURL, Message: msg, }) - return nil, conditions, false + return nil, conditions, false, false + } + + usingProxyForHost, err := c.proxyDetector.UsingProxyForHost(endpointHostPort.Host) + if err != nil { + msg := fmt.Sprintf("%s: %s", "spec.endpoint URL error", err.Error()) + conditions = append(conditions, &metav1.Condition{ + Type: typeEndpointURLValid, + Status: metav1.ConditionFalse, + Reason: reasonInvalidEndpointCannotDetermineProxy, + Message: msg, + }) + return nil, conditions, false, false } conditions = append(conditions, &metav1.Condition{ @@ -495,7 +528,7 @@ func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditi Reason: conditionsutil.ReasonSuccess, Message: "spec.endpoint is a valid URL", }) - return &endpointHostPort, conditions, true + return &endpointHostPort, conditions, usingProxyForHost, true } func (c *webhookCacheFillerController) updateStatus( diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index e75e51607..4b0b5fa0c 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -44,8 +44,10 @@ import ( "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/mocks/mockcachevalue" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/proxydetect" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/conditionstestutil" + "go.pinniped.dev/internal/testutil/fakeproxydetect" "go.pinniped.dev/internal/testutil/tlsserver" ) @@ -302,6 +304,11 @@ func TestController(t *testing.T) { Message: "successfully dialed webhook server", } } + happyWebhookConnectionValidWithoutDialingDueToProxy := func(time metav1.Time, observedGeneration int64) metav1.Condition { + c := happyWebhookConnectionValid(time, observedGeneration) + c.Message = "skipped dialing connection probe because HTTPS_PROXY is configured for use with the specified host" + return c + } unknownWebhookConnectionValid := func(time metav1.Time, observedGeneration int64) metav1.Condition { return metav1.Condition{ Type: "WebhookConnectionValid", @@ -373,7 +380,16 @@ func TestController(t *testing.T) { Message: fmt.Sprintf(`spec.endpoint URL %s has invalid scheme, require 'https'`, endpoint), } } - + sadEndpointURLValidProxyDetectErr := func(msg string, time metav1.Time, observedGeneration int64) metav1.Condition { + return metav1.Condition{ + Type: "EndpointURLValid", + Status: "False", + ObservedGeneration: observedGeneration, + LastTransitionTime: time, + Reason: "InvalidEndpointCannotDetermineProxy", + Message: msg, + } + } sadEndpointURLValidWithMessage := func(time metav1.Time, observedGeneration int64, msg string) metav1.Condition { return metav1.Condition{ Type: "EndpointURLValid", @@ -412,10 +428,11 @@ func TestController(t *testing.T) { webhookAuthenticators []runtime.Object secretsAndConfigMaps []runtime.Object // for modifying the clients to hack in arbitrary api responses - configClient func(*conciergefake.Clientset) - wantSyncErr testutil.RequireErrorStringFunc - wantLogLines []string - wantActions func() []coretesting.Action + configClient func(*conciergefake.Clientset) + proxyDetector func(t *testing.T) proxydetect.ProxyDetect + wantSyncErr testutil.RequireErrorStringFunc + wantLogLines []string + wantActions func() []coretesting.Action // random comment so lines above don't have huge indents wantNamesOfWebhookAuthenticatorsInCache []string }{ @@ -1011,6 +1028,80 @@ func TestController(t *testing.T) { }, wantNamesOfWebhookAuthenticatorsInCache: []string{"test-name"}, // keeps the old entry in the cache }, + { + name: "Sync: previously cached valid authenticator with unchanged endpoint URL and CA bundle hash has invalid status conditions in informer cache, as can happen on subsequent sync soon after multiple quick status updates (when the informer cache finally catches up), and the webhook host would be reached through a proxy: should update status in current sync", + cache: func(t *testing.T, cache *authncache.Cache) { + oldCA, err := base64.StdEncoding.DecodeString(goodWebhookAuthenticatorSpecWithCA.TLS.CertificateAuthorityData) + require.NoError(t, err) + cache.Store( + authncache.Key{ + Name: "test-name", + Kind: "WebhookAuthenticator", + APIGroup: authenticationv1alpha1.SchemeGroupVersion.Group, + }, + newCacheValue(t, goodWebhookAuthenticatorSpecWithCA, string(oldCA)), + ) + }, + webhookAuthenticators: []runtime.Object{ + &authenticationv1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + Generation: 1234, + }, + Spec: goodWebhookAuthenticatorSpecWithCA, + Status: authenticationv1alpha1.WebhookAuthenticatorStatus{ + Conditions: conditionstestutil.Replace( + allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0), + []metav1.Condition{ + sadTLSConfigurationValid(frozenMetav1Now, 0), + unknownWebhookConnectionValid(frozenMetav1Now, 0), + unknownAuthenticatorValid(frozenMetav1Now, 0), + sadReadyCondition(frozenMetav1Now, 0), + }, + ), + Phase: "Error", + }, + }, + }, + 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, "127.0.0.1", 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":"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 +1112,15 @@ 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, "127.0.0.1", 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 +1145,104 @@ 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, "127.0.0.1", 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, "127.0.0.1", 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 +2123,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 +2140,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 +2386,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 +2450,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..93b0cd1f0 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,8 +57,9 @@ const ( GitHubConnectionValid string = "GitHubConnectionValid" ClaimsValid string = "ClaimsValid" - reasonInvalid = "Invalid" - reasonInvalidHost = "InvalidHost" + reasonInvalid = "Invalid" + reasonInvalidHost = "InvalidHost" + reasonUnableToMakeRequest = "UnableToMakeRequest" apiDotGithubDotCom = "api.github.com" githubDotCom = "github.com" @@ -107,6 +107,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 +118,7 @@ type gitHubWatcherController struct { secretInformer corev1informers.SecretInformer configMapInformer corev1informers.ConfigMapInformer clock clock.Clock - dialer ptls.Dialer + probeURLFunc ProbeURLFunc validatedCache GitHubValidatedAPICacheI } @@ -131,7 +133,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 +145,7 @@ func New( secretInformer: secretInformer, configMapInformer: configMapInformer, clock: clock, - dialer: dialer, + probeURLFunc: probeURLFunc, validatedCache: NewGitHubValidatedAPICache(validatedCache), } @@ -299,7 +301,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 +367,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 +390,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 +473,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 +509,43 @@ 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. +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 + } + + 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..c56f30c91 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,27 @@ 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) - -type allowNoDials struct { - t *testing.T -} - -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 +209,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 +220,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 +230,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 +241,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 +256,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 +267,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 +278,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 +289,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 +305,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 +316,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 +326,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 +383,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 +403,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 +475,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 +537,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 +552,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 +612,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 +627,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 +687,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 +702,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 +762,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 +777,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 +837,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 +852,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 +912,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 +989,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 +1149,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 +1157,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 +1165,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 +1320,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 +1328,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 +1336,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 +1395,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 +1714,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 +1728,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 +1797,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 +1811,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 +1843,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 +1857,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 +1898,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 +1939,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 +1980,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 +2021,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 +2062,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 +2103,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 +2144,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 +2185,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 +2228,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 +2271,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 +2314,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 +2357,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 +2400,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 +2428,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 +2448,7 @@ func TestController(t *testing.T) { logger, controllerlib.WithInformer, frozenClockForLastTransitionTime, - dialer, + probeURLFunc, validatedCache, ) @@ -2722,7 +2659,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 +2771,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 +2831,7 @@ func TestController_OnlyWantActions(t *testing.T) { logger, controllerlib.WithInformer, frozenClockForLastTransitionTime, - ptls.NewDialer(), + ProbeURL, cache.NewExpiring(), ) @@ -3018,7 +2955,7 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { logger, observableInformers.WithInformer, clock.RealClock{}, - ptls.NewDialer(), + nil, cache.NewExpiring(), ) @@ -3075,7 +3012,7 @@ func TestGitHubUpstreamWatcherControllerFilterConfigMaps(t *testing.T) { logger, observableInformers.WithInformer, clock.RealClock{}, - ptls.NewDialer(), + nil, cache.NewExpiring(), ) @@ -3132,7 +3069,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..eaa2e99fe --- /dev/null +++ b/internal/proxydetect/proxydetect.go @@ -0,0 +1,56 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package proxydetect + +import ( + "context" + "fmt" + "net/http" + "net/url" +) + +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{proxyFromEnvironmentFunc: 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..160b10da5 --- /dev/null +++ b/internal/proxydetect/proxydetect_test.go @@ -0,0 +1,77 @@ +// 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 TestProxyDetect(t *testing.T) { + t.Parallel() + + 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) + + require.Equal(t, tt.wantProxied, proxied) + if tt.wantErr != "" { + require.Equal(t, tt.wantErr, err.Error()) + } else { + require.NoError(t, err) + } + }) + } +} 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_webhookauthenticator_status_test.go b/test/integration/concierge_webhookauthenticator_status_test.go index c5a9dec3c..985c3e19e 100644 --- a/test/integration/concierge_webhookauthenticator_status_test.go +++ b/test/integration/concierge_webhookauthenticator_status_test.go @@ -153,14 +153,16 @@ 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", @@ -168,10 +170,15 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { 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", + maybeSkip: func(t *testing.T) { + if conciergeUsingHTTPSProxy { + t.Skip("Skipping test that requires HTTPS_PROXY to be unset") + } + }, spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { caBundleString := "invalid base64-encoded data" webhookSpec := env.TestWebhook.DeepCopy() @@ -182,7 +189,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, finalConditions: replaceSomeConditions( - allSuccessfulWebhookAuthenticatorConditions(), + allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { Type: "Ready", @@ -210,6 +217,11 @@ 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 { + t.Skip("Skipping test that requires HTTPS_PROXY to be unset") + } + }, spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { webhookSpec := env.TestWebhook.DeepCopy() webhookSpec.TLS = &authenticationv1alpha1.TLSSpec{ @@ -219,7 +231,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, finalConditions: replaceSomeConditions( - allSuccessfulWebhookAuthenticatorConditions(), + allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { Type: "Ready", @@ -242,6 +254,11 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { }, { name: "invalid with unresponsive endpoint will result in a WebhookAuthenticator that is not ready", + maybeSkip: func(t *testing.T) { + if conciergeUsingHTTPSProxy { + t.Skip("Skipping test that requires HTTPS_PROXY to be unset") + } + }, spec: func() *authenticationv1alpha1.WebhookAuthenticatorSpec { webhookSpec := env.TestWebhook.DeepCopy() webhookSpec.TLS = &authenticationv1alpha1.TLSSpec{ @@ -252,7 +269,7 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) { }, initialPhase: authenticationv1alpha1.WebhookAuthenticatorPhaseError, finalConditions: replaceSomeConditions( - allSuccessfulWebhookAuthenticatorConditions(), + allSuccessfulWebhookAuthenticatorConditions(false), []metav1.Condition{ { Type: "Ready", @@ -397,7 +414,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 +449,7 @@ func allSuccessfulWebhookAuthenticatorConditions() []metav1.Condition { Type: "WebhookConnectionValid", Status: "True", Reason: "Success", - Message: "successfully dialed webhook server", + Message: dialMessage, }, } } 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 +}