Refactor active directory and ldap controllers to share almost everything

Signed-off-by: Ryan Richard <richardry@vmware.com>

internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go

@@ -6,12 +6,8 @@ package activedirectoryupstreamwatcher
 
 import (
 	"context"
-	"crypto/x509"
-	"encoding/base64"
 	"fmt"
-	"time"
 
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -26,28 +22,109 @@ import (
 	"go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/oidc/provider"
-	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/upstreamldap"
 )
 
 const (
-	activeDirectoryControllerName        = "active-directory-upstream-observer"
+	activeDirectoryControllerName = "active-directory-upstream-observer"
-	activeDirectoryBindAccountSecretType = corev1.SecretTypeBasicAuth
-	testActiveDirectoryConnectionTimeout = 90 * time.Second
-
-	// Constants related to conditions.
-	typeBindSecretValid                  = "BindSecretValid"
-	typeTLSConfigurationValid            = "TLSConfigurationValid"
-	typeActiveDirectoryConnectionValid   = "ActiveDirectoryConnectionValid"
-	reasonActiveDirectoryConnectionError = "ActiveDirectoryConnectionError"
-	noTLSConfigurationMessage            = "no TLS configuration provided"
-	loadedTLSConfigurationMessage        = "loaded TLS configuration"
 
 	// Default values for active directory config.
 	defaultActiveDirectoryUsernameAttributeName = "sAMAccountName"
 	defaultActiveDirectoryUIDAttributeName      = "objectGUID"
 )
 
+type activeDirectoryUpstreamGenericLDAPImpl struct {
+	activeDirectoryIdentityProvider v1alpha1.ActiveDirectoryIdentityProvider
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPImpl) Spec() upstreamwatchers.UpstreamGenericLDAPSpec {
+	return &activeDirectoryUpstreamGenericLDAPSpec{g.activeDirectoryIdentityProvider}
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPImpl) Namespace() string {
+	return g.activeDirectoryIdentityProvider.Namespace
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPImpl) Name() string {
+	return g.activeDirectoryIdentityProvider.Name
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPImpl) Generation() int64 {
+	return g.activeDirectoryIdentityProvider.Generation
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus {
+	return &activeDirectoryUpstreamGenericLDAPStatus{g.activeDirectoryIdentityProvider}
+}
+
+type activeDirectoryUpstreamGenericLDAPSpec struct {
+	activeDirectoryIdentityProvider v1alpha1.ActiveDirectoryIdentityProvider
+}
+
+func (s *activeDirectoryUpstreamGenericLDAPSpec) Host() string {
+	return s.activeDirectoryIdentityProvider.Spec.Host
+}
+
+func (s *activeDirectoryUpstreamGenericLDAPSpec) TLSSpec() *v1alpha1.TLSSpec {
+	return s.activeDirectoryIdentityProvider.Spec.TLS
+}
+
+func (s *activeDirectoryUpstreamGenericLDAPSpec) BindSecretName() string {
+	return s.activeDirectoryIdentityProvider.Spec.Bind.SecretName
+}
+
+func (s *activeDirectoryUpstreamGenericLDAPSpec) UserSearch() upstreamwatchers.UpstreamGenericLDAPUserSearch {
+	return &activeDirectoryUpstreamGenericLDAPUserSearch{s.activeDirectoryIdentityProvider.Spec.UserSearch}
+}
+
+func (s *activeDirectoryUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers.UpstreamGenericLDAPGroupSearch {
+	return &activeDirectoryUpstreamGenericLDAPGroupSearch{s.activeDirectoryIdentityProvider.Spec.GroupSearch}
+}
+
+type activeDirectoryUpstreamGenericLDAPUserSearch struct {
+	userSearch v1alpha1.ActiveDirectoryIdentityProviderUserSearch
+}
+
+func (u *activeDirectoryUpstreamGenericLDAPUserSearch) Base() string {
+	return u.userSearch.Base
+}
+
+func (u *activeDirectoryUpstreamGenericLDAPUserSearch) Filter() string {
+	return u.userSearch.Filter
+}
+
+func (u *activeDirectoryUpstreamGenericLDAPUserSearch) UsernameAttribute() string {
+	return u.userSearch.Attributes.Username
+}
+
+func (u *activeDirectoryUpstreamGenericLDAPUserSearch) UIDAttribute() string {
+	return u.userSearch.Attributes.UID
+}
+
+type activeDirectoryUpstreamGenericLDAPGroupSearch struct {
+	groupSearch v1alpha1.ActiveDirectoryIdentityProviderGroupSearch
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) Base() string {
+	return g.groupSearch.Base
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) Filter() string {
+	return g.groupSearch.Filter
+}
+
+func (g *activeDirectoryUpstreamGenericLDAPGroupSearch) GroupNameAttribute() string {
+	return g.groupSearch.Attributes.GroupName
+}
+
+type activeDirectoryUpstreamGenericLDAPStatus struct {
+	activeDirectoryIdentityProvider v1alpha1.ActiveDirectoryIdentityProvider
+}
+
+func (s *activeDirectoryUpstreamGenericLDAPStatus) Conditions() []v1alpha1.Condition {
+	return s.activeDirectoryIdentityProvider.Status.Conditions
+}
+
 // UpstreamActiveDirectoryIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations.
 type UpstreamActiveDirectoryIdentityProviderICache interface {
 	SetActiveDirectoryIdentityProviders([]provider.UpstreamLDAPIdentityProviderI)
@@ -55,28 +132,13 @@ type UpstreamActiveDirectoryIdentityProviderICache interface {
 
 type activeDirectoryWatcherController struct {
 	cache                                   UpstreamActiveDirectoryIdentityProviderICache
-	validatedSecretVersionsCache            *secretVersionCache
+	validatedSecretVersionsCache            *upstreamwatchers.SecretVersionCache
 	ldapDialer                              upstreamldap.LDAPDialer
 	client                                  pinnipedclientset.Interface
 	activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer
 	secretInformer                          corev1informers.SecretInformer
 }
 
-// An in-memory cache with an entry for each ActiveDirectoryIdentityProvider, to keep track of which ResourceVersion
-// of the bind Secret and which TLS/StartTLS setting was used during the most recent successful validation.
-type secretVersionCache struct {
-	ValidatedSettingsByName map[string]validatedSettings
-}
-
-type validatedSettings struct {
-	BindSecretResourceVersion string
-	LDAPConnectionProtocol    upstreamldap.LDAPConnectionProtocol
-}
-
-func newSecretVersionCache() *secretVersionCache {
-	return &secretVersionCache{ValidatedSettingsByName: map[string]validatedSettings{}}
-}
-
 // New instantiates a new controllerlib.Controller which will populate the provided UpstreamActiveDirectoryIdentityProviderICache.
 func New(
 	idpCache UpstreamActiveDirectoryIdentityProviderICache,
@@ -88,7 +150,7 @@ func New(
 	return newInternal(
 		idpCache,
 		// start with an empty secretVersionCache
-		newSecretVersionCache(),
+		upstreamwatchers.NewSecretVersionCache(),
 		// nil means to use a real production dialer when creating objects to add to the cache
 		nil,
 		client,
@@ -101,7 +163,7 @@ func New(
 // For test dependency injection purposes.
 func newInternal(
 	idpCache UpstreamActiveDirectoryIdentityProviderICache,
-	validatedSecretVersionsCache *secretVersionCache,
+	validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache,
 	ldapDialer upstreamldap.LDAPDialer,
 	client pinnipedclientset.Interface,
 	activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer,
@@ -125,7 +187,7 @@ func newInternal(
 		),
 		withInformer(
 			secretInformer,
-			pinnipedcontroller.MatchAnySecretOfTypeFilter(activeDirectoryBindAccountSecretType, pinnipedcontroller.SingletonQueue()),
+			pinnipedcontroller.MatchAnySecretOfTypeFilter(upstreamwatchers.LDAPBindAccountSecretType, pinnipedcontroller.SingletonQueue()),
 			controllerlib.InformerOption{},
 		),
 	)
@@ -187,212 +249,11 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context,
 		Dialer: c.ldapDialer,
 	}
 
-	conditions := []*v1alpha1.Condition{}
+	conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &activeDirectoryUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSecretVersionsCache, config)
-	secretValidCondition, currentSecretVersion := c.validateSecret(upstream, config)
-	tlsValidCondition := c.validateTLSConfig(upstream, config)
-	conditions = append(conditions, secretValidCondition, tlsValidCondition)
 
-	// No point in trying to connect to the server if the config was already determined to be invalid.
+	c.updateStatus(ctx, upstream, conditions.Conditions())
-	var finishedConfigCondition *v1alpha1.Condition
-	if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue {
-		finishedConfigCondition = c.validateFinishedConfig(ctx, upstream, config, currentSecretVersion)
-		if finishedConfigCondition != nil {
-			conditions = append(conditions, finishedConfigCondition)
-		}
-	}
 
-	c.updateStatus(ctx, upstream, conditions)
+	return upstreamwatchers.EvaluateConditions(conditions, config)
-
-	switch {
-	case secretValidCondition.Status != v1alpha1.ConditionTrue || tlsValidCondition.Status != v1alpha1.ConditionTrue:
-		// Invalid provider, so do not load it into the cache.
-		p = nil
-		requeue = true
-	case finishedConfigCondition != nil && finishedConfigCondition.Status != v1alpha1.ConditionTrue:
-		// Error but load it into the cache anyway, treating this condition failure more like a warning.
-		p = upstreamldap.New(*config)
-		// Try again hoping that the condition will improve.
-		requeue = true
-	default:
-		// Fully validated provider, so load it into the cache.
-		p = upstreamldap.New(*config)
-		requeue = false
-	}
-
-	return p, requeue
-}
-
-func (c *activeDirectoryWatcherController) validateTLSConfig(upstream *v1alpha1.ActiveDirectoryIdentityProvider, config *upstreamldap.ProviderConfig) *v1alpha1.Condition {
-	tlsSpec := upstream.Spec.TLS
-	if tlsSpec == nil {
-		return c.validTLSCondition(noTLSConfigurationMessage)
-	}
-	if len(tlsSpec.CertificateAuthorityData) == 0 {
-		return c.validTLSCondition(loadedTLSConfigurationMessage)
-	}
-
-	bundle, err := base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData)
-	if err != nil {
-		return c.invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", err.Error()))
-	}
-
-	ca := x509.NewCertPool()
-	ok := ca.AppendCertsFromPEM(bundle)
-	if !ok {
-		return c.invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", upstreamwatchers.ErrNoCertificates))
-	}
-
-	config.CABundle = bundle
-	return c.validTLSCondition(loadedTLSConfigurationMessage)
-}
-
-func (c *activeDirectoryWatcherController) validateFinishedConfig(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider, config *upstreamldap.ProviderConfig, currentSecretVersion string) *v1alpha1.Condition {
-	if c.hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream, currentSecretVersion, config) {
-		return nil
-	}
-
-	testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, testActiveDirectoryConnectionTimeout)
-	defer cancelFunc()
-
-	condition := c.testConnection(testConnectionTimeout, upstream, config, currentSecretVersion)
-
-	if condition.Status == v1alpha1.ConditionTrue {
-		// Remember (in-memory for this pod) that the controller has successfully validated the LDAP provider
-		// using this version of the Secret. This is for performance reasons, to avoid attempting to connect to
-		// the LDAP server more than is needed. If the pod restarts, it will attempt this validation again.
-		c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()] = validatedSettings{
-			BindSecretResourceVersion: currentSecretVersion,
-			LDAPConnectionProtocol:    config.ConnectionProtocol,
-		}
-	}
-
-	return condition
-}
-
-func (c *activeDirectoryWatcherController) testConnection(
-	ctx context.Context,
-	upstream *v1alpha1.ActiveDirectoryIdentityProvider,
-	config *upstreamldap.ProviderConfig,
-	currentSecretVersion string,
-) *v1alpha1.Condition {
-	// First try using TLS.
-	config.ConnectionProtocol = upstreamldap.TLS
-	tlsLDAPProvider := upstreamldap.New(*config)
-	err := tlsLDAPProvider.TestConnection(ctx)
-	if err != nil {
-		plog.InfoErr("testing LDAP connection using TLS failed, so trying again with StartTLS", err, "host", config.Host)
-		// If there was any error, try again with StartTLS instead.
-		config.ConnectionProtocol = upstreamldap.StartTLS
-		startTLSLDAPProvider := upstreamldap.New(*config)
-		startTLSErr := startTLSLDAPProvider.TestConnection(ctx)
-		if startTLSErr == nil {
-			plog.Info("testing LDAP connection using StartTLS succeeded", "host", config.Host)
-			// Successfully able to fall back to using StartTLS, so clear the original
-			// error and consider the connection test to be successful.
-			err = nil
-		} else {
-			plog.InfoErr("testing LDAP connection using StartTLS also failed", err, "host", config.Host)
-			// Falling back to StartTLS also failed, so put TLS back into the config
-			// and consider the connection test to be failed.
-			config.ConnectionProtocol = upstreamldap.TLS
-		}
-	}
-
-	if err != nil {
-		return &v1alpha1.Condition{
-			Type:   typeActiveDirectoryConnectionValid,
-			Status: v1alpha1.ConditionFalse,
-			Reason: reasonActiveDirectoryConnectionError,
-			Message: fmt.Sprintf(`could not successfully connect to "%s" and bind as user "%s": %s`,
-				config.Host, config.BindUsername, err.Error()),
-		}
-	}
-
-	return &v1alpha1.Condition{
-		Type:   typeActiveDirectoryConnectionValid,
-		Status: v1alpha1.ConditionTrue,
-		Reason: upstreamwatchers.ReasonSuccess,
-		Message: fmt.Sprintf(`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
-			config.Host, config.BindUsername, upstream.Spec.Bind.SecretName, currentSecretVersion),
-	}
-}
-
-func (c *activeDirectoryWatcherController) hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream *v1alpha1.ActiveDirectoryIdentityProvider, currentSecretVersion string, config *upstreamldap.ProviderConfig) bool {
-	currentGeneration := upstream.Generation
-	for _, cond := range upstream.Status.Conditions {
-		if cond.Type == typeActiveDirectoryConnectionValid && cond.Status == v1alpha1.ConditionTrue && cond.ObservedGeneration == currentGeneration {
-			// Found a previously successful condition for the current spec generation.
-			// Now figure out which version of the bind Secret was used during that previous validation, if any.
-			validatedSecretVersion := c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()]
-			if validatedSecretVersion.BindSecretResourceVersion == currentSecretVersion {
-				// Reload the TLS vs StartTLS setting that was previously validated.
-				config.ConnectionProtocol = validatedSecretVersion.LDAPConnectionProtocol
-				return true
-			}
-		}
-	}
-	return false
-}
-
-func (c *activeDirectoryWatcherController) validTLSCondition(message string) *v1alpha1.Condition {
-	return &v1alpha1.Condition{
-		Type:    typeTLSConfigurationValid,
-		Status:  v1alpha1.ConditionTrue,
-		Reason:  upstreamwatchers.ReasonSuccess,
-		Message: message,
-	}
-}
-
-func (c *activeDirectoryWatcherController) invalidTLSCondition(message string) *v1alpha1.Condition {
-	return &v1alpha1.Condition{
-		Type:    typeTLSConfigurationValid,
-		Status:  v1alpha1.ConditionFalse,
-		Reason:  upstreamwatchers.ReasonInvalidTLSConfig,
-		Message: message,
-	}
-}
-
-func (c *activeDirectoryWatcherController) validateSecret(upstream *v1alpha1.ActiveDirectoryIdentityProvider, config *upstreamldap.ProviderConfig) (*v1alpha1.Condition, string) {
-	secretName := upstream.Spec.Bind.SecretName
-
-	secret, err := c.secretInformer.Lister().Secrets(upstream.Namespace).Get(secretName)
-	if err != nil {
-		return &v1alpha1.Condition{
-			Type:    typeBindSecretValid,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  upstreamwatchers.ReasonNotFound,
-			Message: err.Error(),
-		}, ""
-	}
-
-	if secret.Type != corev1.SecretTypeBasicAuth {
-		return &v1alpha1.Condition{
-			Type:   typeBindSecretValid,
-			Status: v1alpha1.ConditionFalse,
-			Reason: upstreamwatchers.ReasonWrongType,
-			Message: fmt.Sprintf("referenced Secret %q has wrong type %q (should be %q)",
-				secretName, secret.Type, corev1.SecretTypeBasicAuth),
-		}, secret.ResourceVersion
-	}
-
-	config.BindUsername = string(secret.Data[corev1.BasicAuthUsernameKey])
-	config.BindPassword = string(secret.Data[corev1.BasicAuthPasswordKey])
-	if len(config.BindUsername) == 0 || len(config.BindPassword) == 0 {
-		return &v1alpha1.Condition{
-			Type:   typeBindSecretValid,
-			Status: v1alpha1.ConditionFalse,
-			Reason: upstreamwatchers.ReasonMissingKeys,
-			Message: fmt.Sprintf("referenced Secret %q is missing required keys %q",
-				secretName, []string{corev1.BasicAuthUsernameKey, corev1.BasicAuthPasswordKey}),
-		}, secret.ResourceVersion
-	}
-
-	return &v1alpha1.Condition{
-		Type:    typeBindSecretValid,
-		Status:  v1alpha1.ConditionTrue,
-		Reason:  upstreamwatchers.ReasonSuccess,
-		Message: "loaded bind secret",
-	}, secret.ResourceVersion
 }
 
 func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.ActiveDirectoryIdentityProvider, conditions []*v1alpha1.Condition) {

internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go

@@ -25,6 +25,7 @@ import (
 	pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
 	pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
 	"go.pinniped.dev/internal/certauthority"
+	"go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/endpointaddr"
 	"go.pinniped.dev/internal/mocks/mockldapconn"
@@ -235,7 +236,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 	}
 	activeDirectoryConnectionValidTrueCondition := func(gen int64, secretVersion string) v1alpha1.Condition {
 		return v1alpha1.Condition{
-			Type:               "ActiveDirectoryConnectionValid",
+			Type:               "LDAPConnectionValid",
 			Status:             "True",
 			LastTransitionTime: now,
 			Reason:             "Success",
@@ -257,8 +258,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 	}
 	allConditionsTrue := func(gen int64, secretVersion string) []v1alpha1.Condition {
 		return []v1alpha1.Condition{
-			activeDirectoryConnectionValidTrueCondition(gen, secretVersion),
 			bindSecretValidTrueCondition(gen),
+			activeDirectoryConnectionValidTrueCondition(gen, secretVersion),
 			tlsConfigurationValidLoadedTrueCondition(gen),
 		}
 	}
@@ -273,7 +274,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 
 	tests := []struct {
 		name                     string
-		initialValidatedSettings map[string]validatedSettings
+		initialValidatedSettings map[string]upstreamwatchers.ValidatedSettings
 		inputUpstreams           []runtime.Object
 		inputSecrets             []runtime.Object
 		setupMocks               func(conn *mockldapconn.MockConn)
@@ -281,7 +282,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 		wantErr                  string
 		wantResultingCache       []*upstreamldap.ProviderConfig
 		wantResultingUpstreams   []v1alpha1.ActiveDirectoryIdentityProvider
-		wantValidatedSettings    map[string]validatedSettings
+		wantValidatedSettings    map[string]upstreamwatchers.ValidatedSettings
 	}{
 		{
 			name:               "no ActiveDirectoryIdentityProvider upstreams clears the cache",
@@ -304,7 +305,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name:               "missing secret",
@@ -474,8 +475,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{
 					Phase: "Ready",
 					Conditions: []v1alpha1.Condition{
-						activeDirectoryConnectionValidTrueCondition(1234, "4242"),
 						bindSecretValidTrueCondition(1234),
+						activeDirectoryConnectionValidTrueCondition(1234, "4242"),
 						{
 							Type:               "TLSConfigurationValid",
 							Status:             "True",
@@ -487,7 +488,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when TLS connection fails it tries to use StartTLS instead: without a specified port it automatically switches ports",
@@ -530,8 +531,9 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{
 					Phase: "Ready",
 					Conditions: []v1alpha1.Condition{
+						bindSecretValidTrueCondition(1234),
 						{
-							Type:               "ActiveDirectoryConnectionValid",
+							Type:               "LDAPConnectionValid",
 							Status:             "True",
 							LastTransitionTime: now,
 							Reason:             "Success",
@@ -540,12 +542,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 								"ldap.example.com", testBindUsername, testSecretName, "4242"),
 							ObservedGeneration: 1234,
 						},
-						bindSecretValidTrueCondition(1234),
 						tlsConfigurationValidLoadedTrueCondition(1234),
 					},
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
 		},
 		{
 			name: "when TLS connection fails it tries to use StartTLS instead: with a specified port it does not automatically switch ports",
@@ -587,17 +588,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{
 					Phase: "Error",
 					Conditions: []v1alpha1.Condition{
+						bindSecretValidTrueCondition(1234),
 						{
-							Type:               "ActiveDirectoryConnectionValid",
+							Type:               "LDAPConnectionValid",
 							Status:             "False",
 							LastTransitionTime: now,
-							Reason:             "ActiveDirectoryConnectionError",
+							Reason:             "LDAPConnectionError",
 							Message: fmt.Sprintf(
 								`could not successfully connect to "%s" and bind as user "%s": error dialing host "%s": some dial error`,
 								"ldap.example.com:5678", testBindUsername, "ldap.example.com:5678"),
 							ObservedGeneration: 1234,
 						},
-						bindSecretValidTrueCondition(1234),
 						tlsConfigurationValidLoadedTrueCondition(1234),
 					},
 				},
@@ -642,7 +643,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream",
@@ -685,7 +686,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				},
 			},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name:           "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway (treated like a warning)",
@@ -704,17 +705,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				Status: v1alpha1.ActiveDirectoryIdentityProviderStatus{
 					Phase: "Error",
 					Conditions: []v1alpha1.Condition{
+						bindSecretValidTrueCondition(1234),
 						{
-							Type:               "ActiveDirectoryConnectionValid",
+							Type:               "LDAPConnectionValid",
 							Status:             "False",
 							LastTransitionTime: now,
-							Reason:             "ActiveDirectoryConnectionError",
+							Reason:             "LDAPConnectionError",
 							Message: fmt.Sprintf(
 								`could not successfully connect to "%s" and bind as user "%s": error binding as "%s": some bind error`,
 								testHost, testBindUsername, testBindUsername),
 							ObservedGeneration: 1234,
 						},
-						bindSecretValidTrueCondition(1234),
 						tlsConfigurationValidLoadedTrueCondition(1234),
 					},
 				},
@@ -729,7 +730,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
 			},
@@ -741,7 +742,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when the LDAP server connection was already validated using StartTLS for the current resource generation and secret version, then do not validate it again and keep using StartTLS",
@@ -752,7 +753,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
 			},
@@ -764,7 +765,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
 		},
 		{
 			name: "when the LDAP server connection was validated for an older resource generation, then try to validate it again",
@@ -775,7 +776,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should perform a test dial and bind.
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
@@ -789,7 +790,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when the LDAP server connection validation previously failed for this resource generation, then try to validate it again",
@@ -797,17 +798,17 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				upstream.Generation = 1234
 				upstream.Status.Conditions = []v1alpha1.Condition{
 					{
-						Type:               "ActiveDirectoryConnectionValid",
+						Type:               "LDAPConnectionValid",
 						Status:             "False", // failure!
 						LastTransitionTime: now,
-						Reason:             "ActiveDirectoryConnectionError",
+						Reason:             "LDAPConnectionError",
 						Message:            "some-error-message",
 						ObservedGeneration: 1234, // same (current) generation!
 					},
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should perform a test dial and bind.
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
@@ -821,7 +822,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when the LDAP server connection was already validated for this resource generation but the bind secret has changed, then try to validate it again",
@@ -831,8 +832,8 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					activeDirectoryConnectionValidTrueCondition(1234, "4241"), // same spec generation, old secret version
 				}
 			})},
-			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},                                                                         // newer secret version!
+			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},                                                                                          // newer secret version!
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4241", LDAPConnectionProtocol: upstreamldap.TLS}}, // old version was validated
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4241", LDAPConnectionProtocol: upstreamldap.TLS}}, // old version was validated
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should perform a test dial and bind.
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
@@ -846,7 +847,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when the input activedirectoryidentityprovider leaves user attributes blank, provide default values",
@@ -887,7 +888,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 	}
 
@@ -923,7 +924,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 				return conn, nil
 			})}
 
-			validatedSecretVersionCache := newSecretVersionCache()
+			validatedSecretVersionCache := upstreamwatchers.NewSecretVersionCache()
 			if tt.initialValidatedSettings != nil {
 				validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings
 			}
@@ -977,7 +978,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 
 			// Check that the controller remembered which version of the secret it most recently validated successfully with.
 			if tt.wantValidatedSettings == nil {
-				tt.wantValidatedSettings = map[string]validatedSettings{}
+				tt.wantValidatedSettings = map[string]upstreamwatchers.ValidatedSettings{}
 			}
 			require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName)
 		})

internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go

@@ -6,8 +6,6 @@ package ldapupstreamwatcher
 
 import (
 	"context"
-	"crypto/x509"
-	"encoding/base64"
 	"fmt"
 	"time"
 
@@ -26,7 +24,6 @@ import (
 	"go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/oidc/provider"
-	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/upstreamldap"
 )
 
@@ -34,16 +31,100 @@ const (
 	ldapControllerName        = "ldap-upstream-observer"
 	ldapBindAccountSecretType = corev1.SecretTypeBasicAuth
 	testLDAPConnectionTimeout = 90 * time.Second
-
-	// Constants related to conditions.
-	typeBindSecretValid           = "BindSecretValid"
-	typeTLSConfigurationValid     = "TLSConfigurationValid"
-	typeLDAPConnectionValid       = "LDAPConnectionValid"
-	reasonLDAPConnectionError     = "LDAPConnectionError"
-	noTLSConfigurationMessage     = "no TLS configuration provided"
-	loadedTLSConfigurationMessage = "loaded TLS configuration"
 )
 
+type ldapUpstreamGenericLDAPImpl struct {
+	ldapIdentityProvider v1alpha1.LDAPIdentityProvider
+}
+
+func (g *ldapUpstreamGenericLDAPImpl) Spec() upstreamwatchers.UpstreamGenericLDAPSpec {
+	return &ldapUpstreamGenericLDAPSpec{g.ldapIdentityProvider}
+}
+
+func (g *ldapUpstreamGenericLDAPImpl) Namespace() string {
+	return g.ldapIdentityProvider.Namespace
+}
+
+func (g *ldapUpstreamGenericLDAPImpl) Name() string {
+	return g.ldapIdentityProvider.Name
+}
+
+func (g *ldapUpstreamGenericLDAPImpl) Generation() int64 {
+	return g.ldapIdentityProvider.Generation
+}
+
+func (g *ldapUpstreamGenericLDAPImpl) Status() upstreamwatchers.UpstreamGenericLDAPStatus {
+	return &ldapUpstreamGenericLDAPStatus{g.ldapIdentityProvider}
+}
+
+type ldapUpstreamGenericLDAPSpec struct {
+	ldapIdentityProvider v1alpha1.LDAPIdentityProvider
+}
+
+func (s *ldapUpstreamGenericLDAPSpec) Host() string {
+	return s.ldapIdentityProvider.Spec.Host
+}
+
+func (s *ldapUpstreamGenericLDAPSpec) TLSSpec() *v1alpha1.TLSSpec {
+	return s.ldapIdentityProvider.Spec.TLS
+}
+
+func (s *ldapUpstreamGenericLDAPSpec) BindSecretName() string {
+	return s.ldapIdentityProvider.Spec.Bind.SecretName
+}
+
+func (s *ldapUpstreamGenericLDAPSpec) UserSearch() upstreamwatchers.UpstreamGenericLDAPUserSearch {
+	return &ldapUpstreamGenericLDAPUserSearch{s.ldapIdentityProvider.Spec.UserSearch}
+}
+
+func (s *ldapUpstreamGenericLDAPSpec) GroupSearch() upstreamwatchers.UpstreamGenericLDAPGroupSearch {
+	return &ldapUpstreamGenericLDAPGroupSearch{s.ldapIdentityProvider.Spec.GroupSearch}
+}
+
+type ldapUpstreamGenericLDAPUserSearch struct {
+	userSearch v1alpha1.LDAPIdentityProviderUserSearch
+}
+
+func (u *ldapUpstreamGenericLDAPUserSearch) Base() string {
+	return u.userSearch.Base
+}
+
+func (u *ldapUpstreamGenericLDAPUserSearch) Filter() string {
+	return u.userSearch.Filter
+}
+
+func (u *ldapUpstreamGenericLDAPUserSearch) UsernameAttribute() string {
+	return u.userSearch.Attributes.Username
+}
+
+func (u *ldapUpstreamGenericLDAPUserSearch) UIDAttribute() string {
+	return u.userSearch.Attributes.UID
+}
+
+type ldapUpstreamGenericLDAPGroupSearch struct {
+	groupSearch v1alpha1.LDAPIdentityProviderGroupSearch
+}
+
+func (g *ldapUpstreamGenericLDAPGroupSearch) Base() string {
+	return g.groupSearch.Base
+}
+
+func (g *ldapUpstreamGenericLDAPGroupSearch) Filter() string {
+	return g.groupSearch.Filter
+}
+
+func (g *ldapUpstreamGenericLDAPGroupSearch) GroupNameAttribute() string {
+	return g.groupSearch.Attributes.GroupName
+}
+
+type ldapUpstreamGenericLDAPStatus struct {
+	ldapIdentityProvider v1alpha1.LDAPIdentityProvider
+}
+
+func (s *ldapUpstreamGenericLDAPStatus) Conditions() []v1alpha1.Condition {
+	return s.ldapIdentityProvider.Status.Conditions
+}
+
 // UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations.
 type UpstreamLDAPIdentityProviderICache interface {
 	SetLDAPIdentityProviders([]provider.UpstreamLDAPIdentityProviderI)
@@ -51,28 +132,13 @@ type UpstreamLDAPIdentityProviderICache interface {
 
 type ldapWatcherController struct {
 	cache                        UpstreamLDAPIdentityProviderICache
-	validatedSecretVersionsCache *secretVersionCache
+	validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache
 	ldapDialer                   upstreamldap.LDAPDialer
 	client                       pinnipedclientset.Interface
 	ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer
 	secretInformer               corev1informers.SecretInformer
 }
 
-// An in-memory cache with an entry for each LDAPIdentityProvider, to keep track of which ResourceVersion
-// of the bind Secret and which TLS/StartTLS setting was used during the most recent successful validation.
-type secretVersionCache struct {
-	ValidatedSettingsByName map[string]validatedSettings
-}
-
-type validatedSettings struct {
-	BindSecretResourceVersion string
-	LDAPConnectionProtocol    upstreamldap.LDAPConnectionProtocol
-}
-
-func newSecretVersionCache() *secretVersionCache {
-	return &secretVersionCache{ValidatedSettingsByName: map[string]validatedSettings{}}
-}
-
 // New instantiates a new controllerlib.Controller which will populate the provided UpstreamLDAPIdentityProviderICache.
 func New(
 	idpCache UpstreamLDAPIdentityProviderICache,
@@ -84,7 +150,7 @@ func New(
 	return newInternal(
 		idpCache,
 		// start with an empty secretVersionCache
-		newSecretVersionCache(),
+		upstreamwatchers.NewSecretVersionCache(),
 		// nil means to use a real production dialer when creating objects to add to the cache
 		nil,
 		client,
@@ -97,7 +163,7 @@ func New(
 // For test dependency injection purposes.
 func newInternal(
 	idpCache UpstreamLDAPIdentityProviderICache,
-	validatedSecretVersionsCache *secretVersionCache,
+	validatedSecretVersionsCache *upstreamwatchers.SecretVersionCache,
 	ldapDialer upstreamldap.LDAPDialer,
 	client pinnipedclientset.Interface,
 	ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer,
@@ -174,212 +240,11 @@ func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream *
 		Dialer: c.ldapDialer,
 	}
 
-	conditions := []*v1alpha1.Condition{}
+	conditions := upstreamwatchers.ValidateGenericLDAP(ctx, &ldapUpstreamGenericLDAPImpl{*upstream}, c.secretInformer, c.validatedSecretVersionsCache, config)
-	secretValidCondition, currentSecretVersion := c.validateSecret(upstream, config)
-	tlsValidCondition := c.validateTLSConfig(upstream, config)
-	conditions = append(conditions, secretValidCondition, tlsValidCondition)
 
-	// No point in trying to connect to the server if the config was already determined to be invalid.
+	c.updateStatus(ctx, upstream, conditions.Conditions())
-	var finishedConfigCondition *v1alpha1.Condition
-	if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue {
-		finishedConfigCondition = c.validateFinishedConfig(ctx, upstream, config, currentSecretVersion)
-		if finishedConfigCondition != nil {
-			conditions = append(conditions, finishedConfigCondition)
-		}
-	}
 
-	c.updateStatus(ctx, upstream, conditions)
+	return upstreamwatchers.EvaluateConditions(conditions, config)
-
-	switch {
-	case secretValidCondition.Status != v1alpha1.ConditionTrue || tlsValidCondition.Status != v1alpha1.ConditionTrue:
-		// Invalid provider, so do not load it into the cache.
-		p = nil
-		requeue = true
-	case finishedConfigCondition != nil && finishedConfigCondition.Status != v1alpha1.ConditionTrue:
-		// Error but load it into the cache anyway, treating this condition failure more like a warning.
-		p = upstreamldap.New(*config)
-		// Try again hoping that the condition will improve.
-		requeue = true
-	default:
-		// Fully validated provider, so load it into the cache.
-		p = upstreamldap.New(*config)
-		requeue = false
-	}
-
-	return p, requeue
-}
-
-func (c *ldapWatcherController) validateTLSConfig(upstream *v1alpha1.LDAPIdentityProvider, config *upstreamldap.ProviderConfig) *v1alpha1.Condition {
-	tlsSpec := upstream.Spec.TLS
-	if tlsSpec == nil {
-		return c.validTLSCondition(noTLSConfigurationMessage)
-	}
-	if len(tlsSpec.CertificateAuthorityData) == 0 {
-		return c.validTLSCondition(loadedTLSConfigurationMessage)
-	}
-
-	bundle, err := base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData)
-	if err != nil {
-		return c.invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", err.Error()))
-	}
-
-	ca := x509.NewCertPool()
-	ok := ca.AppendCertsFromPEM(bundle)
-	if !ok {
-		return c.invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", upstreamwatchers.ErrNoCertificates))
-	}
-
-	config.CABundle = bundle
-	return c.validTLSCondition(loadedTLSConfigurationMessage)
-}
-
-func (c *ldapWatcherController) validateFinishedConfig(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider, config *upstreamldap.ProviderConfig, currentSecretVersion string) *v1alpha1.Condition {
-	if c.hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream, currentSecretVersion, config) {
-		return nil
-	}
-
-	testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, testLDAPConnectionTimeout)
-	defer cancelFunc()
-
-	condition := c.testConnection(testConnectionTimeout, upstream, config, currentSecretVersion)
-
-	if condition.Status == v1alpha1.ConditionTrue {
-		// Remember (in-memory for this pod) that the controller has successfully validated the LDAP provider
-		// using this version of the Secret. This is for performance reasons, to avoid attempting to connect to
-		// the LDAP server more than is needed. If the pod restarts, it will attempt this validation again.
-		c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()] = validatedSettings{
-			BindSecretResourceVersion: currentSecretVersion,
-			LDAPConnectionProtocol:    config.ConnectionProtocol,
-		}
-	}
-
-	return condition
-}
-
-func (c *ldapWatcherController) testConnection(
-	ctx context.Context,
-	upstream *v1alpha1.LDAPIdentityProvider,
-	config *upstreamldap.ProviderConfig,
-	currentSecretVersion string,
-) *v1alpha1.Condition {
-	// First try using TLS.
-	config.ConnectionProtocol = upstreamldap.TLS
-	tlsLDAPProvider := upstreamldap.New(*config)
-	err := tlsLDAPProvider.TestConnection(ctx)
-	if err != nil {
-		plog.InfoErr("testing LDAP connection using TLS failed, so trying again with StartTLS", err, "host", config.Host)
-		// If there was any error, try again with StartTLS instead.
-		config.ConnectionProtocol = upstreamldap.StartTLS
-		startTLSLDAPProvider := upstreamldap.New(*config)
-		startTLSErr := startTLSLDAPProvider.TestConnection(ctx)
-		if startTLSErr == nil {
-			plog.Info("testing LDAP connection using StartTLS succeeded", "host", config.Host)
-			// Successfully able to fall back to using StartTLS, so clear the original
-			// error and consider the connection test to be successful.
-			err = nil
-		} else {
-			plog.InfoErr("testing LDAP connection using StartTLS also failed", err, "host", config.Host)
-			// Falling back to StartTLS also failed, so put TLS back into the config
-			// and consider the connection test to be failed.
-			config.ConnectionProtocol = upstreamldap.TLS
-		}
-	}
-
-	if err != nil {
-		return &v1alpha1.Condition{
-			Type:   typeLDAPConnectionValid,
-			Status: v1alpha1.ConditionFalse,
-			Reason: reasonLDAPConnectionError,
-			Message: fmt.Sprintf(`could not successfully connect to "%s" and bind as user "%s": %s`,
-				config.Host, config.BindUsername, err.Error()),
-		}
-	}
-
-	return &v1alpha1.Condition{
-		Type:   typeLDAPConnectionValid,
-		Status: v1alpha1.ConditionTrue,
-		Reason: upstreamwatchers.ReasonSuccess,
-		Message: fmt.Sprintf(`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
-			config.Host, config.BindUsername, upstream.Spec.Bind.SecretName, currentSecretVersion),
-	}
-}
-
-func (c *ldapWatcherController) hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream *v1alpha1.LDAPIdentityProvider, currentSecretVersion string, config *upstreamldap.ProviderConfig) bool {
-	currentGeneration := upstream.Generation
-	for _, cond := range upstream.Status.Conditions {
-		if cond.Type == typeLDAPConnectionValid && cond.Status == v1alpha1.ConditionTrue && cond.ObservedGeneration == currentGeneration {
-			// Found a previously successful condition for the current spec generation.
-			// Now figure out which version of the bind Secret was used during that previous validation, if any.
-			validatedSecretVersion := c.validatedSecretVersionsCache.ValidatedSettingsByName[upstream.GetName()]
-			if validatedSecretVersion.BindSecretResourceVersion == currentSecretVersion {
-				// Reload the TLS vs StartTLS setting that was previously validated.
-				config.ConnectionProtocol = validatedSecretVersion.LDAPConnectionProtocol
-				return true
-			}
-		}
-	}
-	return false
-}
-
-func (c *ldapWatcherController) validTLSCondition(message string) *v1alpha1.Condition {
-	return &v1alpha1.Condition{
-		Type:    typeTLSConfigurationValid,
-		Status:  v1alpha1.ConditionTrue,
-		Reason:  upstreamwatchers.ReasonSuccess,
-		Message: message,
-	}
-}
-
-func (c *ldapWatcherController) invalidTLSCondition(message string) *v1alpha1.Condition {
-	return &v1alpha1.Condition{
-		Type:    typeTLSConfigurationValid,
-		Status:  v1alpha1.ConditionFalse,
-		Reason:  upstreamwatchers.ReasonInvalidTLSConfig,
-		Message: message,
-	}
-}
-
-func (c *ldapWatcherController) validateSecret(upstream *v1alpha1.LDAPIdentityProvider, config *upstreamldap.ProviderConfig) (*v1alpha1.Condition, string) {
-	secretName := upstream.Spec.Bind.SecretName
-
-	secret, err := c.secretInformer.Lister().Secrets(upstream.Namespace).Get(secretName)
-	if err != nil {
-		return &v1alpha1.Condition{
-			Type:    typeBindSecretValid,
-			Status:  v1alpha1.ConditionFalse,
-			Reason:  upstreamwatchers.ReasonNotFound,
-			Message: err.Error(),
-		}, ""
-	}
-
-	if secret.Type != corev1.SecretTypeBasicAuth {
-		return &v1alpha1.Condition{
-			Type:   typeBindSecretValid,
-			Status: v1alpha1.ConditionFalse,
-			Reason: upstreamwatchers.ReasonWrongType,
-			Message: fmt.Sprintf("referenced Secret %q has wrong type %q (should be %q)",
-				secretName, secret.Type, corev1.SecretTypeBasicAuth),
-		}, secret.ResourceVersion
-	}
-
-	config.BindUsername = string(secret.Data[corev1.BasicAuthUsernameKey])
-	config.BindPassword = string(secret.Data[corev1.BasicAuthPasswordKey])
-	if len(config.BindUsername) == 0 || len(config.BindPassword) == 0 {
-		return &v1alpha1.Condition{
-			Type:   typeBindSecretValid,
-			Status: v1alpha1.ConditionFalse,
-			Reason: upstreamwatchers.ReasonMissingKeys,
-			Message: fmt.Sprintf("referenced Secret %q is missing required keys %q",
-				secretName, []string{corev1.BasicAuthUsernameKey, corev1.BasicAuthPasswordKey}),
-		}, secret.ResourceVersion
-	}
-
-	return &v1alpha1.Condition{
-		Type:    typeBindSecretValid,
-		Status:  v1alpha1.ConditionTrue,
-		Reason:  upstreamwatchers.ReasonSuccess,
-		Message: "loaded bind secret",
-	}, secret.ResourceVersion
 }
 
 func (c *ldapWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider, conditions []*v1alpha1.Condition) {

internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go

@@ -25,6 +25,7 @@ import (
 	pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
 	pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
 	"go.pinniped.dev/internal/certauthority"
+	"go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/endpointaddr"
 	"go.pinniped.dev/internal/mocks/mockldapconn"
@@ -273,7 +274,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 
 	tests := []struct {
 		name                     string
-		initialValidatedSettings map[string]validatedSettings
+		initialValidatedSettings map[string]upstreamwatchers.ValidatedSettings
 		inputUpstreams           []runtime.Object
 		inputSecrets             []runtime.Object
 		setupMocks               func(conn *mockldapconn.MockConn)
@@ -281,7 +282,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 		wantErr                  string
 		wantResultingCache       []*upstreamldap.ProviderConfig
 		wantResultingUpstreams   []v1alpha1.LDAPIdentityProvider
-		wantValidatedSettings    map[string]validatedSettings
+		wantValidatedSettings    map[string]upstreamwatchers.ValidatedSettings
 	}{
 		{
 			name:               "no LDAPIdentityProvider upstreams clears the cache",
@@ -304,7 +305,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name:               "missing secret",
@@ -487,7 +488,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when TLS connection fails it tries to use StartTLS instead: without a specified port it automatically switches ports",
@@ -545,7 +546,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
 		},
 		{
 			name: "when TLS connection fails it tries to use StartTLS instead: with a specified port it does not automatically switch ports",
@@ -642,7 +643,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "one valid upstream and one invalid upstream updates the cache to include only the valid upstream",
@@ -685,7 +686,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					},
 				},
 			},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name:           "when testing the connection to the LDAP server fails then the upstream is still added to the cache anyway (treated like a warning)",
@@ -729,7 +730,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
 			},
@@ -741,7 +742,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when the LDAP server connection was already validated using StartTLS for the current resource generation and secret version, then do not validate it again and keep using StartTLS",
@@ -752,7 +753,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should not perform a test dial and bind. No mocking here means the test will fail if Bind() or Close() are called.
 			},
@@ -764,7 +765,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS}},
 		},
 		{
 			name: "when the LDAP server connection was validated for an older resource generation, then try to validate it again",
@@ -775,7 +776,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should perform a test dial and bind.
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
@@ -789,7 +790,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when the LDAP server connection validation previously failed for this resource generation, then try to validate it again",
@@ -807,7 +808,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				}
 			})},
 			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}},
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "1", LDAPConnectionProtocol: upstreamldap.TLS}},
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should perform a test dial and bind.
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
@@ -821,7 +822,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 		{
 			name: "when the LDAP server connection was already validated for this resource generation but the bind secret has changed, then try to validate it again",
@@ -831,8 +832,8 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					ldapConnectionValidTrueCondition(1234, "4241"), // same spec generation, old secret version
 				}
 			})},
-			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},                                                                         // newer secret version!
+			inputSecrets:             []runtime.Object{validBindUserSecret("4242")},                                                                                          // newer secret version!
-			initialValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4241", LDAPConnectionProtocol: upstreamldap.TLS}}, // old version was validated
+			initialValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4241", LDAPConnectionProtocol: upstreamldap.TLS}}, // old version was validated
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				// Should perform a test dial and bind.
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
@@ -846,7 +847,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 					Conditions: allConditionsTrue(1234, "4242"),
 				},
 			}},
-			wantValidatedSettings: map[string]validatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
+			wantValidatedSettings: map[string]upstreamwatchers.ValidatedSettings{testName: {BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS}},
 		},
 	}
 
@@ -882,7 +883,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 				return conn, nil
 			})}
 
-			validatedSecretVersionCache := newSecretVersionCache()
+			validatedSecretVersionCache := upstreamwatchers.NewSecretVersionCache()
 			if tt.initialValidatedSettings != nil {
 				validatedSecretVersionCache.ValidatedSettingsByName = tt.initialValidatedSettings
 			}
@@ -936,7 +937,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
 
 			// Check that the controller remembered which version of the secret it most recently validated successfully with.
 			if tt.wantValidatedSettings == nil {
-				tt.wantValidatedSettings = map[string]validatedSettings{}
+				tt.wantValidatedSettings = map[string]upstreamwatchers.ValidatedSettings{}
 			}
 			require.Equal(t, tt.wantValidatedSettings, validatedSecretVersionCache.ValidatedSettingsByName)
 		})

internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go

@@ -3,7 +3,22 @@
 
 package upstreamwatchers
 
-import "go.pinniped.dev/internal/constable"
+import (
+	"context"
+	"crypto/x509"
+	"encoding/base64"
+	"fmt"
+	"time"
+
+	corev1 "k8s.io/api/core/v1"
+	corev1informers "k8s.io/client-go/informers/core/v1"
+
+	"go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
+	"go.pinniped.dev/internal/constable"
+	"go.pinniped.dev/internal/oidc/provider"
+	"go.pinniped.dev/internal/plog"
+	"go.pinniped.dev/internal/upstreamldap"
+)
 
 const (
 	ReasonNotFound         = "SecretNotFound"
@@ -13,4 +28,294 @@ const (
 	ReasonInvalidTLSConfig = "InvalidTLSConfig"
 
 	ErrNoCertificates = constable.Error("no certificates found")
+
+	LDAPBindAccountSecretType = corev1.SecretTypeBasicAuth
+	TestLDAPConnectionTimeout = 90 * time.Second
+
+	// Constants related to conditions.
+	typeBindSecretValid           = "BindSecretValid"
+	typeTLSConfigurationValid     = "TLSConfigurationValid"
+	typeLDAPConnectionValid       = "LDAPConnectionValid"
+	reasonLDAPConnectionError     = "LDAPConnectionError"
+	noTLSConfigurationMessage     = "no TLS configuration provided"
+	loadedTLSConfigurationMessage = "loaded TLS configuration"
 )
+
+// An in-memory cache with an entry for each ActiveDirectoryIdentityProvider, to keep track of which ResourceVersion
+// of the bind Secret and which TLS/StartTLS setting was used during the most recent successful validation.
+type SecretVersionCache struct {
+	ValidatedSettingsByName map[string]ValidatedSettings
+}
+
+type ValidatedSettings struct {
+	BindSecretResourceVersion string
+	LDAPConnectionProtocol    upstreamldap.LDAPConnectionProtocol
+}
+
+func NewSecretVersionCache() *SecretVersionCache {
+	return &SecretVersionCache{ValidatedSettingsByName: map[string]ValidatedSettings{}}
+}
+
+// read only interface for sharing between ldap and active directory.
+type UpstreamGenericLDAPIDP interface {
+	Spec() UpstreamGenericLDAPSpec
+	Name() string
+	Namespace() string
+	Generation() int64
+	Status() UpstreamGenericLDAPStatus
+}
+
+type UpstreamGenericLDAPSpec interface {
+	Host() string
+	TLSSpec() *v1alpha1.TLSSpec
+	BindSecretName() string
+	UserSearch() UpstreamGenericLDAPUserSearch
+	GroupSearch() UpstreamGenericLDAPGroupSearch
+}
+
+type UpstreamGenericLDAPUserSearch interface {
+	Base() string
+	Filter() string
+	UsernameAttribute() string
+	UIDAttribute() string
+}
+
+type UpstreamGenericLDAPGroupSearch interface {
+	Base() string
+	Filter() string
+	GroupNameAttribute() string
+}
+
+type UpstreamGenericLDAPStatus interface {
+	Conditions() []v1alpha1.Condition
+}
+
+func ValidateTLSConfig(tlsSpec *v1alpha1.TLSSpec, config *upstreamldap.ProviderConfig) *v1alpha1.Condition {
+	if tlsSpec == nil {
+		return validTLSCondition(noTLSConfigurationMessage)
+	}
+	if len(tlsSpec.CertificateAuthorityData) == 0 {
+		return validTLSCondition(loadedTLSConfigurationMessage)
+	}
+
+	bundle, err := base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData)
+	if err != nil {
+		return invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", err.Error()))
+	}
+
+	ca := x509.NewCertPool()
+	ok := ca.AppendCertsFromPEM(bundle)
+	if !ok {
+		return invalidTLSCondition(fmt.Sprintf("certificateAuthorityData is invalid: %s", ErrNoCertificates))
+	}
+
+	config.CABundle = bundle
+	return validTLSCondition(loadedTLSConfigurationMessage)
+}
+
+func TestConnection(
+	ctx context.Context,
+	bindSecretName string,
+	config *upstreamldap.ProviderConfig,
+	currentSecretVersion string,
+) *v1alpha1.Condition {
+	// First try using TLS.
+	config.ConnectionProtocol = upstreamldap.TLS
+	tlsLDAPProvider := upstreamldap.New(*config)
+	err := tlsLDAPProvider.TestConnection(ctx)
+	if err != nil {
+		plog.InfoErr("testing LDAP connection using TLS failed, so trying again with StartTLS", err, "host", config.Host)
+		// If there was any error, try again with StartTLS instead.
+		config.ConnectionProtocol = upstreamldap.StartTLS
+		startTLSLDAPProvider := upstreamldap.New(*config)
+		startTLSErr := startTLSLDAPProvider.TestConnection(ctx)
+		if startTLSErr == nil {
+			plog.Info("testing LDAP connection using StartTLS succeeded", "host", config.Host)
+			// Successfully able to fall back to using StartTLS, so clear the original
+			// error and consider the connection test to be successful.
+			err = nil
+		} else {
+			plog.InfoErr("testing LDAP connection using StartTLS also failed", err, "host", config.Host)
+			// Falling back to StartTLS also failed, so put TLS back into the config
+			// and consider the connection test to be failed.
+			config.ConnectionProtocol = upstreamldap.TLS
+		}
+	}
+
+	if err != nil {
+		return &v1alpha1.Condition{
+			Type:   typeLDAPConnectionValid,
+			Status: v1alpha1.ConditionFalse,
+			Reason: reasonLDAPConnectionError,
+			Message: fmt.Sprintf(`could not successfully connect to "%s" and bind as user "%s": %s`,
+				config.Host, config.BindUsername, err.Error()),
+		}
+	}
+
+	return &v1alpha1.Condition{
+		Type:   typeLDAPConnectionValid,
+		Status: v1alpha1.ConditionTrue,
+		Reason: ReasonSuccess,
+		Message: fmt.Sprintf(`successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`,
+			config.Host, config.BindUsername, bindSecretName, currentSecretVersion),
+	}
+}
+
+func HasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(secretVersionCache *SecretVersionCache, currentGeneration int64, upstreamStatusConditions []v1alpha1.Condition, upstreamName string, currentSecretVersion string, config *upstreamldap.ProviderConfig) bool {
+	for _, cond := range upstreamStatusConditions {
+		if cond.Type == typeLDAPConnectionValid && cond.Status == v1alpha1.ConditionTrue && cond.ObservedGeneration == currentGeneration {
+			// Found a previously successful condition for the current spec generation.
+			// Now figure out which version of the bind Secret was used during that previous validation, if any.
+			validatedSecretVersion := secretVersionCache.ValidatedSettingsByName[upstreamName]
+			if validatedSecretVersion.BindSecretResourceVersion == currentSecretVersion {
+				// Reload the TLS vs StartTLS setting that was previously validated.
+				config.ConnectionProtocol = validatedSecretVersion.LDAPConnectionProtocol
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func validTLSCondition(message string) *v1alpha1.Condition {
+	return &v1alpha1.Condition{
+		Type:    typeTLSConfigurationValid,
+		Status:  v1alpha1.ConditionTrue,
+		Reason:  ReasonSuccess,
+		Message: message,
+	}
+}
+
+func invalidTLSCondition(message string) *v1alpha1.Condition {
+	return &v1alpha1.Condition{
+		Type:    typeTLSConfigurationValid,
+		Status:  v1alpha1.ConditionFalse,
+		Reason:  ReasonInvalidTLSConfig,
+		Message: message,
+	}
+}
+
+func ValidateSecret(secretInformer corev1informers.SecretInformer, secretName string, secretNamespace string, config *upstreamldap.ProviderConfig) (*v1alpha1.Condition, string) {
+	secret, err := secretInformer.Lister().Secrets(secretNamespace).Get(secretName)
+	if err != nil {
+		return &v1alpha1.Condition{
+			Type:    typeBindSecretValid,
+			Status:  v1alpha1.ConditionFalse,
+			Reason:  ReasonNotFound,
+			Message: err.Error(),
+		}, ""
+	}
+
+	if secret.Type != corev1.SecretTypeBasicAuth {
+		return &v1alpha1.Condition{
+			Type:   typeBindSecretValid,
+			Status: v1alpha1.ConditionFalse,
+			Reason: ReasonWrongType,
+			Message: fmt.Sprintf("referenced Secret %q has wrong type %q (should be %q)",
+				secretName, secret.Type, corev1.SecretTypeBasicAuth),
+		}, secret.ResourceVersion
+	}
+
+	config.BindUsername = string(secret.Data[corev1.BasicAuthUsernameKey])
+	config.BindPassword = string(secret.Data[corev1.BasicAuthPasswordKey])
+	if len(config.BindUsername) == 0 || len(config.BindPassword) == 0 {
+		return &v1alpha1.Condition{
+			Type:   typeBindSecretValid,
+			Status: v1alpha1.ConditionFalse,
+			Reason: ReasonMissingKeys,
+			Message: fmt.Sprintf("referenced Secret %q is missing required keys %q",
+				secretName, []string{corev1.BasicAuthUsernameKey, corev1.BasicAuthPasswordKey}),
+		}, secret.ResourceVersion
+	}
+
+	return &v1alpha1.Condition{
+		Type:    typeBindSecretValid,
+		Status:  v1alpha1.ConditionTrue,
+		Reason:  ReasonSuccess,
+		Message: "loaded bind secret",
+	}, secret.ResourceVersion
+}
+
+type GradatedConditions struct {
+	gradatedConditions []GradatedCondition
+}
+
+func (g *GradatedConditions) Conditions() []*v1alpha1.Condition {
+	conditions := []*v1alpha1.Condition{}
+	for _, gc := range g.gradatedConditions {
+		conditions = append(conditions, gc.condition)
+	}
+	return conditions
+}
+
+func (g *GradatedConditions) Append(condition *v1alpha1.Condition, isFatal bool) {
+	g.gradatedConditions = append(g.gradatedConditions, GradatedCondition{condition: condition, isFatal: isFatal})
+}
+
+// A condition and a boolean that tells you whether it's fatal or just a warning.
+type GradatedCondition struct {
+	condition *v1alpha1.Condition
+	isFatal   bool
+}
+
+func ValidateGenericLDAP(ctx context.Context, upstream UpstreamGenericLDAPIDP, secretInformer corev1informers.SecretInformer, validatedSecretVersionsCache *SecretVersionCache, config *upstreamldap.ProviderConfig) GradatedConditions {
+	conditions := GradatedConditions{}
+	secretValidCondition, currentSecretVersion := ValidateSecret(secretInformer, upstream.Spec().BindSecretName(), upstream.Namespace(), config)
+	conditions.Append(secretValidCondition, true)
+	tlsValidCondition := ValidateTLSConfig(upstream.Spec().TLSSpec(), config)
+	conditions.Append(tlsValidCondition, true)
+
+	// No point in trying to connect to the server if the config was already determined to be invalid.
+	var ldapConnectionValidCondition *v1alpha1.Condition
+	if secretValidCondition.Status == v1alpha1.ConditionTrue && tlsValidCondition.Status == v1alpha1.ConditionTrue {
+		ldapConnectionValidCondition = validateAndSetLDAPServerConnectivity(ctx, validatedSecretVersionsCache, upstream, config, currentSecretVersion)
+		if ldapConnectionValidCondition != nil {
+			conditions.Append(ldapConnectionValidCondition, false)
+		}
+	}
+	return conditions
+}
+
+func validateAndSetLDAPServerConnectivity(ctx context.Context, validatedSecretVersionsCache *SecretVersionCache, upstream UpstreamGenericLDAPIDP, config *upstreamldap.ProviderConfig, currentSecretVersion string) *v1alpha1.Condition {
+	// TODO refactor validateAndSetLDAPServerConnectivity to be shared and take a helper function for the defaultNamingContext stuff
+	//  so that can be shared.
+	if HasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(validatedSecretVersionsCache, upstream.Generation(), upstream.Status().Conditions(), upstream.Name(), currentSecretVersion, config) {
+		return nil
+	}
+
+	testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, TestLDAPConnectionTimeout)
+	defer cancelFunc()
+
+	condition := TestConnection(testConnectionTimeout, upstream.Spec().BindSecretName(), config, currentSecretVersion)
+
+	if condition.Status == v1alpha1.ConditionTrue {
+		// Remember (in-memory for this pod) that the controller has successfully validated the LDAP provider
+		// using this version of the Secret. This is for performance reasons, to avoid attempting to connect to
+		// the LDAP server more than is needed. If the pod restarts, it will attempt this validation again.
+		validatedSecretVersionsCache.ValidatedSettingsByName[upstream.Name()] = ValidatedSettings{
+			BindSecretResourceVersion: currentSecretVersion,
+			LDAPConnectionProtocol:    config.ConnectionProtocol,
+		}
+	}
+
+	return condition
+}
+
+func EvaluateConditions(conditions GradatedConditions, config *upstreamldap.ProviderConfig) (provider.UpstreamLDAPIdentityProviderI, bool) {
+	for _, gradatedCondition := range conditions.gradatedConditions {
+		if gradatedCondition.condition.Status != v1alpha1.ConditionTrue && gradatedCondition.isFatal {
+			// Invalid provider, so do not load it into the cache.
+			return nil, true
+		}
+	}
+
+	for _, gradatedCondition := range conditions.gradatedConditions {
+		if gradatedCondition.condition.Status != v1alpha1.ConditionTrue && !gradatedCondition.isFatal {
+			// Error but load it into the cache anyway, treating this condition failure more like a warning.
+			// Try again hoping that the condition will improve.
+			return upstreamldap.New(*config), true
+		}
+	}
+	// Fully validated provider, so load it into the cache.
+	return upstreamldap.New(*config), false
+}

test/integration/supervisor_login_test.go

@@ -343,7 +343,7 @@ func requireSuccessfulActiveDirectoryIdentityProviderConditions(t *testing.T, ad
 			require.Equal(t, "loaded bind secret", condition.Message)
 		case "TLSConfigurationValid":
 			require.Equal(t, "loaded TLS configuration", condition.Message)
-		case "ActiveDirectoryConnectionValid":
+		case "LDAPConnectionValid":
 			require.Equal(t, expectedActiveDirectoryConnectionValidMessage, condition.Message)
 		}
 	}
@@ -351,7 +351,7 @@ func requireSuccessfulActiveDirectoryIdentityProviderConditions(t *testing.T, ad
 	require.ElementsMatch(t, [][]string{
 		{"BindSecretValid", "True", "Success"},
 		{"TLSConfigurationValid", "True", "Success"},
-		{"ActiveDirectoryConnectionValid", "True", "Success"},
+		{"LDAPConnectionValid", "True", "Success"},
 	}, conditionsSummary)
 }