feat: Initialize connectors lazily (not on server start)

[dlipovetsky]

Overview

Initialize connectors lazily (not on server start).

What this PR does / why we need it

Dex initializes all connectors on server start. Connectors may fail to initialize, e.g. if they require a remote service that is unavailable, or if they are misconfigured. If any connector fails to initialize, dex exits. This PR adds an option to skip connector initialization during server start. Dex will initialize each connector when it needs it to serve a request.

This change is backward-compatible. The current behavior (exiting) remains the default; the new behavior is enabled by a field in the configuration file:

lazyInitConnectors: true

Closes #1723

Special notes for your reviewer

Does this PR introduce a user-facing change?

Add a configuration option to start the server without initializing connectors.

[msdolbey]

👍 ❤️

[dlipovetsky]

@nabokihms Could you please allow the CI workflows to run? Thanks!

[dlipovetsky]

The PR is backward compatible, addresses a long-standing issue, comes with a unit test, and CI is passing (ne job fails because the PR needs some labels, which I don't have the permission to create)

@nabokihms and/or @sagikazarmark, please let me know if there's anything else I can do to help it along, or if you disagree with the approach I took. Thank you! 🙏

[nabokihms]

Sorry, slacked a little because of KubeCon. I will take a look till the end of the week and give my feedback.

[nabokihms]

@dlipovetsky Sorry for the long delay. This feature overlaps with what we in Palak also need, so I'd be happy to work with you on this feature.

I had a chance to look through the code and even test the PR, and there are some questions I'd like to discuss upfront.

1. A connector is available from the start if it is a lazily initialized connector. Thus, users can see the connector on the list of available connectors and click on it, attempting to authenticate with it (despite Dex knowing that the connector is not ready).

   This does not seem like a valid UX to me. Connectors should not be pickable until initialized.

2. Each failed attempt is a request to the external provider. Dex will try to make a request to the provider without exponential backoff or any other protection mechanisms in case of unavailability.

3. If all connectors are lazily initialized, it is possible to end up with a working Dex without working connectors, which is misleading. Dex may be working but not ready to serve to auth requests.

What do you think about the following proposal? This is more complicated, but I believe provides better UX for users.

• Initialize connectors in the background in a loop if the first initialization attempt fails (like Kubernetes does).
• Do not show the connector in the list of connectors until initialized.
• Fail the readiness probe if there are no initialized connectors.

[dlipovetsky]

@dlipovetsky Sorry for the long delay. This feature overlaps with what we in Palak also need, so I'd be happy to work with you on this feature.

Thanks for the review!

1. A connector is available from the start if it is a lazily initialized connector. Thus, users can see the connector on the list of available connectors and click on it, attempting to authenticate with it (despite Dex knowing that the connector is not ready).
   This does not seem like a valid UX to me. Connectors should not be pickable until initialized.

Good point. I do not use the dex UI directly, so I did not see (or consider) this.

2. Each failed attempt is a request to the external provider. Dex will try to make a request to the provider without exponential backoff or any other protection mechanisms in case of unavailability.

Good point. A couple of questions: Does "attempt" mean a login attempt? When you say "Dex will try to make a request to the provider," are you referring to this code path:

dex/server/server.go

Line 639 in 1890bf3

conn, err := s.OpenConnector(storageConnector)

3. If all connectors are lazily initialized, it is possible to end up with a working Dex without working connectors, which is misleading. Dex may be working but not ready to serve to auth requests.

Good point.

What do you think about the following proposal? This is more complicated, but I believe provides better UX for users.

I agree, your proposal does provide better UX. Being unfamiliar with the dex code, I made the smallest possible change that allowed dex to start in the presence of a misconfigured connector 😅

Let me know if you'd like to me to work on this, or if you'd like to do it yourself. Also, let me know if you'd like the work to go in a new PR.

[nabokihms]

@dlipovetsky I would be happy if you had time to work on this feature. It is ok to work on this improvement in the scope of this PR. Opening a new PR is not necessary.