Use Options to conditionally enable endpoints health check. Refactor …

…HTTP client injection to use options as well.

connector/connector.go

@@ -98,11 +98,3 @@ type RefreshConnector interface {
 	// changes since the token was last refreshed.
 	Refresh(ctx context.Context, s Scopes, identity Identity) (Identity, error)
 }
-
-// CheckEndpointConnector is a connector that can test its endpoints for availability.
-type CheckEndpointConnector interface {
-	// CheckEndpoint is called when a client wants to check the availability of
-	// endpoints used by the connector. If no error is returned, the connector
-	// can be assumed to be working.
-	CheckEndpoint(ctx context.Context) error
-}

connector/openshift/openshift.go

@@ -26,7 +26,7 @@ const (
 	usersURLPath     = "/apis/user.openshift.io/v1/users/~"
 )
 
-// Config holds configuration options for OpenShift login
+// Config holds configuration options for OpenShift login.
 type Config struct {
 	Issuer       string   `json:"issuer"`
 	ClientID     string   `json:"clientID"`
@@ -37,24 +37,27 @@ type Config struct {
 	RootCA       string   `json:"rootCA"`
 }
 
+// Option allows overriding default values of the openshift connector.
+type Option func(*openshiftConnector)
+
 var (
-	_ connector.CallbackConnector      = (*openshiftConnector)(nil)
-	_ connector.RefreshConnector       = (*openshiftConnector)(nil)
-	_ connector.CheckEndpointConnector = (*openshiftConnector)(nil)
+	_ connector.CallbackConnector = (*openshiftConnector)(nil)
+	_ connector.RefreshConnector  = (*openshiftConnector)(nil)
 )
 
 type openshiftConnector struct {
-	apiURL       string
-	redirectURI  string
-	clientID     string
-	clientSecret string
-	cancel       context.CancelFunc
-	logger       log.Logger
-	httpClient   *http.Client
-	oauth2Config *oauth2.Config
-	insecureCA   bool
-	rootCA       string
-	groups       []string
+	apiURL               string
+	redirectURI          string
+	clientID             string
+	clientSecret         string
+	cancel               context.CancelFunc
+	logger               log.Logger
+	httpClient           *http.Client
+	oauth2Config         *oauth2.Config
+	insecureCA           bool
+	rootCA               string
+	groups               []string
+	endpointsHealthCheck bool
 }
 
 type user struct {
@@ -65,6 +68,23 @@ type user struct {
 	Groups            []string `json:"groups" protobuf:"bytes,4,rep,name=groups"`
 }
 
+// WithHTTPClient will inject a http.Client for the OpenShift connector. This can be used to
+// add custom client specific settings.
+func WithHTTPClient(httpClient *http.Client) Option {
+	return func(o *openshiftConnector) {
+		o.httpClient = httpClient
+	}
+}
+
+// WithEndpointHealthChecks will enable health checks for OAuth endpoints during creation of the
+// OpenShift connector. If the endpoints are not reachable at the time of creation, the connector
+// will not be created and a respective error returned.
+func WithEndpointHealthChecks() Option {
+	return func(o *openshiftConnector) {
+		o.endpointsHealthCheck = true
+	}
+}
+
 // Open returns a connector which can be used to login users through an upstream
 // OpenShift OAuth2 provider.
 func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, err error) {
@@ -73,20 +93,21 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
 		return nil, fmt.Errorf("failed to create HTTP client: %w", err)
 	}
 
-	return c.OpenWithHTTPClient(id, logger, httpClient)
+	return c.OpenWithOptions(id, logger, WithHTTPClient(httpClient))
 }
 
-// OpenWithHTTPClient returns a connector which can be used to login users through an upstream
-// OpenShift OAuth2 provider. It provides the ability to inject a http.Client.
-func (c *Config) OpenWithHTTPClient(id string, logger log.Logger,
-	httpClient *http.Client,
+// OpenWithOptions returns a connector which can be used to login users through an upstream
+// OpenShift OAuth2 provider. It provides the ability to override or inject values based on
+// the given options.
+func (c *Config) OpenWithOptions(id string, logger log.Logger,
+	options ...Option,
 ) (conn connector.Connector, err error) {
 	ctx, cancel := context.WithCancel(context.Background())
 
 	wellKnownURL := strings.TrimSuffix(c.Issuer, "/") + wellKnownURLPath
 	req, err := http.NewRequest(http.MethodGet, wellKnownURL, nil)
 
-	openshiftConnector := openshiftConnector{
+	openshiftConnector := &openshiftConnector{
 		apiURL:       c.Issuer,
 		cancel:       cancel,
 		clientID:     c.ClientID,
@@ -96,7 +117,10 @@ func (c *Config) OpenWithHTTPClient(id string, logger log.Logger,
 		redirectURI:  c.RedirectURI,
 		rootCA:       c.RootCA,
 		groups:       c.Groups,
-		httpClient:   httpClient,
+	}
+
+	for _, opt := range options {
+		opt(openshiftConnector)
 	}
 
 	var metadata struct {
@@ -128,6 +152,12 @@ func (c *Config) OpenWithHTTPClient(id string, logger log.Logger,
 		RedirectURL: c.RedirectURI,
 	}
 
+	if openshiftConnector.endpointsHealthCheck {
+		if err := openshiftConnector.oauth2EndpointsHealthCheck(ctx); err != nil {
+			return nil, fmt.Errorf("failed healthcheck for openshift oauth endpoints %w", err)
+		}
+	}
+
 	return &openshiftConnector, nil
 }
 
@@ -157,7 +187,7 @@ func (e *oauth2Error) Error() string {
 	return e.error + ": " + e.errorDescription
 }
 
-// HandleCallback parses the request and returns the user's identity
+// HandleCallback parses the request and returns the user's identity.
 func (c *openshiftConnector) HandleCallback(s connector.Scopes,
 	r *http.Request,
 ) (identity connector.Identity, err error) {
@@ -229,7 +259,7 @@ func (c *openshiftConnector) identity(ctx context.Context, s connector.Scopes,
 	return identity, nil
 }
 
-// user function returns the OpenShift user associated with the authenticated user
+// user function returns the OpenShift user associated with the authenticated user.
 func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u user, err error) {
 	url := c.apiURL + usersURLPath
 
@@ -259,11 +289,7 @@ func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u u
 	return u, err
 }
 
-func (c *openshiftConnector) CheckEndpoint(ctx context.Context) error {
-	return c.checkOAuth2EndpointsAvailability(ctx)
-}
-
-func (c *openshiftConnector) checkOAuth2EndpointsAvailability(ctx context.Context) error {
+func (c *openshiftConnector) oauth2EndpointsHealthCheck(ctx context.Context) error {
 	for _, endpoint := range []string{c.oauth2Config.Endpoint.AuthURL, c.oauth2Config.Endpoint.TokenURL} {
 		req, err := http.NewRequest(http.MethodHead, endpoint, nil)
 		if err != nil {

connector/openshift/openshift_test.go

@@ -257,7 +257,7 @@ func TestRefreshIdentityFailure(t *testing.T) {
 	expectEquals(t, connector.Identity{}, identity)
 }
 
-func TestCheckEndpoint(t *testing.T) {
+func TestCheckEndpointsAvailability(t *testing.T) {
 	s := newTestServer(map[string]interface{}{
 		"/oauth/authorize": nil,
 		"/oauth/token":     nil,
@@ -272,12 +272,12 @@ func TestCheckEndpoint(t *testing.T) {
 			AuthURL: s.URL + "/oauth/authorize", TokenURL: s.URL + "/oauth/token",
 		},
 	}}
-	err = oc.CheckEndpoint(context.Background())
+	err = oc.oauth2EndpointsHealthCheck(context.Background())
 	expectNil(t, err)
 
 	s.Close()
 
-	err = oc.CheckEndpoint(context.Background())
+	err = oc.oauth2EndpointsHealthCheck(context.Background())
 	expectNotNil(t, err)
 }