Expand IdentityProvidersFound condition in federation_domain_watcher

Co-authored-by: Ryan Richard <richardry@vmware.com>

internal/controller/supervisorconfig/federation_domain_watcher.go

@@ -11,10 +11,11 @@ import (
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/errors"
+	errorsutil "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/utils/clock"
 
 	configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
@@ -37,18 +38,21 @@ const (
 	typeIssuerIsUnique                = "IssuerIsUnique"
 	typeIdentityProvidersFound        = "IdentityProvidersFound"
 
-	reasonSuccess                    = "Success"
+	reasonSuccess                                     = "Success"
-	reasonNotReady                   = "NotReady"
+	reasonNotReady                                    = "NotReady"
-	reasonUnableToValidate           = "UnableToValidate"
+	reasonUnableToValidate                            = "UnableToValidate"
-	reasonInvalidIssuerURL           = "InvalidIssuerURL"
+	reasonInvalidIssuerURL                            = "InvalidIssuerURL"
-	reasonDuplicateIssuer            = "DuplicateIssuer"
+	reasonDuplicateIssuer                             = "DuplicateIssuer"
-	reasonDifferentSecretRefsFound   = "DifferentSecretRefsFound"
+	reasonDifferentSecretRefsFound                    = "DifferentSecretRefsFound"
-	reasonLegacyConfigurationSuccess = "LegacyConfigurationSuccess"
+	reasonLegacyConfigurationSuccess                  = "LegacyConfigurationSuccess"
+	reasonLegacyConfigurationIdentityProviderNotFound = "LegacyConfigurationIdentityProviderNotFound"
+	reasonIdentityProvidersObjectRefsNotFound         = "IdentityProvidersObjectRefsNotFound"
+	reasonIdentityProviderNotSpecified                = "IdentityProviderNotSpecified"
 
 	celTransformerMaxExpressionRuntime = 5 * time.Second
 )
 
-// FederationDomainsSetter can be notified of all known valid providers with its SetIssuer function.
+// FederationDomainsSetter can be notified of all known valid providers with its SetFederationDomains function.
 // If there are no longer any valid issuers, then it can be called with no arguments.
 // Implementations of this type should be thread-safe to support calls from multiple goroutines.
 type FederationDomainsSetter interface {
@@ -155,7 +159,9 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
 
 			// Check if that there is exactly one IDP defined in the Supervisor namespace of any IDP CRD type.
 			idpCRsCount := len(oidcIdentityProviders) + len(ldapIdentityProviders) + len(activeDirectoryIdentityProviders)
-			if idpCRsCount == 1 {
+
+			switch {
+			case idpCRsCount == 1:
 				foundIDPName := ""
 				// If so, default that IDP's DisplayName to be the same as its resource Name.
 				defaultFederationDomainIdentityProvider = &federationdomainproviders.FederationDomainIdentityProvider{}
@@ -173,7 +179,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
 					defaultFederationDomainIdentityProvider.UID = activeDirectoryIdentityProviders[0].UID
 					foundIDPName = activeDirectoryIdentityProviders[0].Name
 				}
-				// Backwards compatibility mode always uses an empty identity transformation pipline since no
+				// Backwards compatibility mode always uses an empty identity transformation pipeline since no
 				// transformations are defined on the FederationDomain.
 				defaultFederationDomainIdentityProvider.Transforms = idtransform.NewTransformationPipeline()
 				conditions = append(conditions, &configv1alpha1.Condition{
@@ -185,42 +191,34 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
 						"identity provider: please explicitly list identity providers in .spec.identityProviders "+
 						"(this legacy configuration mode may be removed in a future version of Pinniped)", foundIDPName),
 				})
-				plog.Warning("detected FederationDomain identity provider backwards compatibility mode: using the one existing identity provider for authentication",
+			case idpCRsCount > 1:
-					"federationDomain", federationDomain.Name)
+				conditions = append(conditions, &configv1alpha1.Condition{
-			} else {
+					Type:   typeIdentityProvidersFound,
-				// TODO(BEN): add the "falsy" status versions of this condition and add tests to cover.
+					Status: configv1alpha1.ConditionFalse,
-				//  this matches the above "truthy" version of the condition above.
+					Reason: reasonIdentityProviderNotSpecified, // vs LegacyConfigurationIdentityProviderNotFound as this is more specific
-				//
+					Message: fmt.Sprintf("no resources were specified by .spec.identityProviders[].objectRef "+
-				// There are no IDP CRs or there is more than one IDP CR. Either way, we are not in the backwards
+						"and %q identity provider resources have been found: "+
-				// compatibility mode because there is not exactly one IDP CR in the namespace, despite the fact that no
+						"please update .spec.identityProviders to specify which identity providers "+
-				// IDPs are listed on the FederationDomain. Create a FederationDomain which has no IDPs and therefore
+						"this federation domain should use", idpCRsCount),
-				// cannot actually be used to log in, but still serves endpoints.
+				})
-				// TODO: Write something into the FederationDomain's status to explain what's happening and how to fix it.
+			default:
-				//   write code for these two condition~
+				conditions = append(conditions, &configv1alpha1.Condition{
-				// Type: IdentityProvidersFound
+					Type:   typeIdentityProvidersFound,
-				// Reason: LegacyConfigurationIdentityProviderNotFound
+					Status: configv1alpha1.ConditionFalse,
-				// Message: "no resources were specified by .spec.identityProviders[].objectRef and {number.of.idp.resources} identity provider resources have been found: please update .spec.identityProviders to specify which identity providers this federation domain should use"
+					Reason: reasonLegacyConfigurationIdentityProviderNotFound,
-				// Status: false
+					Message: "no resources were specified by .spec.identityProviders[].objectRef and no identity provider " +
-				//
+						"resources have been found: please create an identity provider resource",
-				// Type: IdentityProvidersFound
+				})
-				// Reason: LegacyConfigurationIdentityProviderNotFound
-				// Message: "no resources were specified by .spec.identityProviders[].objectRef and no identity provider resources have been found: please create an identity provider resource"
-				// status: false
-				//
-				plog.Warning("FederationDomain has no identity providers listed and there is not exactly one identity provider defined in the namespace: authentication disabled",
-					"federationDomain", federationDomain.Name,
-					"namespace", federationDomain.Namespace,
-					"identityProvidersCustomResourcesCount", idpCRsCount,
-				)
 			}
 		}
 
 		// If there is an explicit list of IDPs on the FederationDomain, then process the list.
 		celTransformer, _ := celtransformer.NewCELTransformer(celTransformerMaxExpressionRuntime) // TODO: what is a good duration limit here?
-		// TODO: handle err
+		// TODO: handle err from NewCELTransformer() above
-		for _, idp := range federationDomain.Spec.IdentityProviders {
+
+		idpNotFoundIndices := []int{}
+		for index, idp := range federationDomain.Spec.IdentityProviders {
 			var idpResourceUID types.UID
-			var idpResourceName string
 			// TODO: Validate that all displayNames are unique within this FederationDomain's spec's list of identity providers.
 			// TODO: Validate that idp.ObjectRef.APIGroup is the expected APIGroup for IDP CRs "idp.supervisor.pinniped.dev"
 			// Validate that each objectRef resolves to an existing IDP. It does not matter if the IDP itself
@@ -228,29 +226,35 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
 			// that does not resolve, put an error on the FederationDomain status.
 			switch idp.ObjectRef.Kind {
 			case "LDAPIdentityProvider":
-				ldapIDP, _ := c.ldapIdentityProviderInformer.Lister().LDAPIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
+				ldapIDP, err := c.ldapIdentityProviderInformer.Lister().LDAPIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
-				// TODO: handle notfound err and also unexpected errors
+				if err == nil {
-				idpResourceName = ldapIDP.Name
+					idpResourceUID = ldapIDP.UID
-				idpResourceUID = ldapIDP.UID
+				} else if errors.IsNotFound(err) {
+					idpNotFoundIndices = append(idpNotFoundIndices, index)
+				} else {
+					// TODO: handle unexpected errors
+				}
 			case "ActiveDirectoryIdentityProvider":
-				adIDP, _ := c.activeDirectoryIdentityProviderInformer.Lister().ActiveDirectoryIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
+				adIDP, err := c.activeDirectoryIdentityProviderInformer.Lister().ActiveDirectoryIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
-				// TODO: handle notfound err and also unexpected errors
+				if err == nil {
-				idpResourceName = adIDP.Name
+					idpResourceUID = adIDP.UID
-				idpResourceUID = adIDP.UID
+				} else if errors.IsNotFound(err) {
+					idpNotFoundIndices = append(idpNotFoundIndices, index)
+				} else {
+					// TODO: handle unexpected errors
+				}
 			case "OIDCIdentityProvider":
-				oidcIDP, _ := c.oidcIdentityProviderInformer.Lister().OIDCIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
+				oidcIDP, err := c.oidcIdentityProviderInformer.Lister().OIDCIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
-				// TODO: handle notfound err and also unexpected errors
+				if err == nil {
-				idpResourceName = oidcIDP.Name
+					idpResourceUID = oidcIDP.UID
-				idpResourceUID = oidcIDP.UID
+				} else if errors.IsNotFound(err) {
+					idpNotFoundIndices = append(idpNotFoundIndices, index)
+				} else {
+					// TODO: handle unexpected errors
+				}
 			default:
-				// TODO: handle bad user input
+				// TODO: handle an IDP type that we do not understand.
 			}
-			plog.Debug("resolved identity provider object reference",
-				"kind", idp.ObjectRef.Kind,
-				"name", idp.ObjectRef.Name,
-				"foundResourceName", idpResourceName,
-				"foundResourceUID", idpResourceUID,
-			)
 
 			// Prepare the transformations.
 			pipeline := idtransform.NewTransformationPipeline()
@@ -383,18 +387,31 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
 				"identityProviderResourceUID", idpResourceUID,
 			)
 		}
-		// TODO:
+
-		// - for the new "non-legacy" version of this to pass, we need to do this work.  however, we don't need this for all of the tests for legacy functionality to work.
+		if len(idpNotFoundIndices) != 0 {
-		// Type: IdentityProvidersFound
+			msgs := []string{}
-		// Reason: IdentityProvidersObjectRefsNotFound
+			for _, idpIndex := range idpNotFoundIndices {
-		// Message: ".spec.identityProviders[].objectRef identifies resource(s) that cannot be found: {list.of.specific.resources.that.cant.be.found.in.case.some.can.be.found.but.not.all}"
+				idp := federationDomain.Spec.IdentityProviders[idpIndex]
-		// Status: false
+				displayName := idp.DisplayName
-		//
+				msgs = append(msgs, fmt.Sprintf("IDP with displayName %q at index %d", displayName, idpIndex))
-		// TODO: maaaaybe we need this happy case as well for the legacy functionality tests to fully pass?
+			}
-		// Type: IdentityProvidersFound
+			conditions = append(conditions, &configv1alpha1.Condition{
-		// Reason: Success
+				Type:    typeIdentityProvidersFound,
-		// Message: Non-legacy happy state "the resources specified by .spec.identityProviders[].objectRef were found"
+				Status:  configv1alpha1.ConditionFalse,
-		// Status: true
+				Reason:  reasonIdentityProvidersObjectRefsNotFound,
+				Message: fmt.Sprintf(".spec.identityProviders[].objectRef identifies resource(s) that cannot be found: %s", strings.Join(msgs, ", ")),
+			})
+		} else {
+			// TODO: write tests for this case.
+			// if len(federationDomain.Spec.IdentityProviders) != 0 {
+			// 	conditions = append(conditions, &configv1alpha1.Condition{
+			// 		Type:    typeIdentityProvidersFound,
+			// 		Status:  configv1alpha1.ConditionTrue,
+			// 		Reason:  reasonSuccess,
+			// 		Message: "the resources specified by .spec.identityProviders[].objectRef were found",
+			// 	})
+			// }
+		}
 
 		// Now that we have the list of IDPs for this FederationDomain, create the issuer.
 		var federationDomainIssuer *federationdomainproviders.FederationDomainIssuer
@@ -439,7 +456,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
 	// (in test, this is wantFederationDomainIssues)
 	c.federationDomainsSetter.SetFederationDomains(federationDomainIssuers...)
 
-	return errors.NewAggregate(errs)
+	return errorsutil.NewAggregate(errs)
 }
 
 func (c *federationDomainWatcherController) updateStatus(

internal/controller/supervisorconfig/federation_domain_watcher_test.go

@@ -19,6 +19,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	coretesting "k8s.io/client-go/testing"
 	clocktesting "k8s.io/utils/clock/testing"
+	"k8s.io/utils/pointer"
 
 	configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
 	idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
@@ -121,6 +122,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 	t.Parallel()
 
 	const namespace = "some-namespace"
+	const apiGroupSupervisor = "idp.supervisor.pinniped.dev"
 
 	frozenNow := time.Date(2020, time.September, 23, 7, 42, 0, 0, time.Local)
 	frozenMetav1Now := metav1.NewTime(frozenNow)
@@ -131,10 +133,16 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 		Resource: "federationdomains",
 	}
 
-	identityProvider := &idpv1alpha1.OIDCIdentityProvider{
+	identityProvider1 := &idpv1alpha1.OIDCIdentityProvider{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "some-name",
+			Name: "some-name1",
-			UID:  "some-uid",
+			UID:  "some-uid1",
+		},
+	}
+	identityProvider2 := &idpv1alpha1.OIDCIdentityProvider{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "some-name2",
+			UID:  "some-uid2",
 		},
 	}
 
@@ -300,7 +308,42 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 		}
 	}
 
-	// sadIdentityProvidersFoundConditionForSomeReasons := func() {}
+	sadIdentityProvidersFoundConditionLegacyConfigurationIdentityProviderNotFound := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
+		return configv1alpha1.Condition{
+			Type:               "IdentityProvidersFound",
+			Status:             "False",
+			ObservedGeneration: observedGeneration,
+			LastTransitionTime: time,
+			Reason:             "LegacyConfigurationIdentityProviderNotFound",
+			Message: "no resources were specified by .spec.identityProviders[].objectRef and no identity provider " +
+				"resources have been found: please create an identity provider resource",
+		}
+	}
+
+	sadIdentityProvidersFoundConditionIdentityProviderNotSpecified := func(idpCRsCount int, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
+		return configv1alpha1.Condition{
+			Type:               "IdentityProvidersFound",
+			Status:             "False",
+			ObservedGeneration: observedGeneration,
+			LastTransitionTime: time,
+			Reason:             "IdentityProviderNotSpecified",
+			Message: fmt.Sprintf("no resources were specified by .spec.identityProviders[].objectRef "+
+				"and %q identity provider resources have been found: "+
+				"please update .spec.identityProviders to specify which identity providers "+
+				"this federation domain should use", idpCRsCount),
+		}
+	}
+
+	sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound := func(msg string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
+		return configv1alpha1.Condition{
+			Type:               "IdentityProvidersFound",
+			Status:             "False",
+			ObservedGeneration: observedGeneration,
+			LastTransitionTime: time,
+			Reason:             "IdentityProvidersObjectRefsNotFound",
+			Message:            msg,
+		}
+	}
 
 	allHappyConditionsLegacyConfigurationSuccess := func(issuer, idpName string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition {
 		return []configv1alpha1.Condition{
@@ -347,25 +390,18 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			},
 		},
 		{
-			// TODO: fill in these conditions in my TODO blocks.
-			// 			conditions = append(conditions, &configv1alpha1.Condition{
-			// 	Type:    typeIssuerURLValid,
-			// 	Status:  configv1alpha1.ConditionFalse,
-			// 	Reason:  reasonInvalidIssuerURL,
-			// 	Message: err.Error(),
-			// })
 			name: "legacy config: when no identity provider is specified on federation domains, but exactly one identity " +
 				"provider resource exists on cluster, the controller will set a default IDP on each federation domain " +
 				"matching the only identity provider found",
 			inputObjects: []runtime.Object{
 				federationDomain1,
 				federationDomain2,
-				identityProvider,
+				identityProvider1,
 			},
 			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
 				return []*federationdomainproviders.FederationDomainIssuer{
-					federationDomainIssuerWithDefaultIDP(t, federationDomain1.Spec.Issuer, identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, federationDomain1.Spec.Issuer, identityProvider1.ObjectMeta),
-					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider1.ObjectMeta),
 				}
 			},
 			wantActions: func(t *testing.T) []coretesting.Action {
@@ -373,13 +409,13 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain1.Namespace,
 						newCopyWithStatus(federationDomain1,
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain2.Namespace,
 						newCopyWithStatus(federationDomain2,
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 				}
@@ -389,16 +425,16 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			name: "when there are two valid FederationDomains, but one is already up to date, the sync loop only updates " +
 				"the out-of-date FederationDomain",
 			inputObjects: []runtime.Object{
-				identityProvider,
+				identityProvider1,
 				newCopyWithStatus(federationDomain1, configv1alpha1.FederationDomainPhaseReady,
-					allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+					allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 				),
 				federationDomain2,
 			},
 			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
 				return []*federationdomainproviders.FederationDomainIssuer{
-					federationDomainIssuerWithDefaultIDP(t, federationDomain1.Spec.Issuer, identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, federationDomain1.Spec.Issuer, identityProvider1.ObjectMeta),
-					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider1.ObjectMeta),
 				}
 			},
 			wantActions: func(t *testing.T) []coretesting.Action {
@@ -406,7 +442,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain2.Namespace,
 						newCopyWithStatus(federationDomain2,
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 				}
@@ -417,7 +453,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			inputObjects: []runtime.Object{
 				federationDomain1,
 				federationDomain2,
-				identityProvider,
+				identityProvider1,
 			},
 			configPinnipedClient: func(client *pinnipedfake.Clientset) {
 				client.PrependReactor(
@@ -436,7 +472,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
 				return []*federationdomainproviders.FederationDomainIssuer{
 					// federationDomain1 is not included because it encountered an error
-					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider1.ObjectMeta),
 				}
 			},
 			wantActions: func(t *testing.T) []coretesting.Action {
@@ -444,13 +480,13 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain1.Namespace,
 						newCopyWithStatus(federationDomain1,
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain2.Namespace,
 						newCopyWithStatus(federationDomain2,
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 				}
@@ -462,12 +498,12 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			inputObjects: []runtime.Object{
 				invalidFederationDomain,
 				federationDomain2,
-				identityProvider,
+				identityProvider1,
 			},
 			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
 				return []*federationdomainproviders.FederationDomainIssuer{
 					// only the valid FederationDomain
-					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider1.ObjectMeta),
 				}
 			},
 			wantActions: func(t *testing.T) []coretesting.Action {
@@ -476,7 +512,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						newCopyWithStatus(invalidFederationDomain,
 							configv1alpha1.FederationDomainPhaseError,
 							[]configv1alpha1.Condition{
-								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider.Name, frozenMetav1Now, 123),
+								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider1.Name, frozenMetav1Now, 123),
 								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
 								sadIssuerURLValidConditionCannotHaveQuery(frozenMetav1Now, 123),
 								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
@@ -487,7 +523,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain2.Namespace,
 						newCopyWithStatus(federationDomain2,
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 				}
@@ -499,7 +535,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			inputObjects: []runtime.Object{
 				invalidFederationDomain,
 				federationDomain2,
-				identityProvider,
+				identityProvider1,
 			},
 			configPinnipedClient: func(client *pinnipedfake.Clientset) {
 				client.PrependReactor(
@@ -518,7 +554,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
 				return []*federationdomainproviders.FederationDomainIssuer{
 					// only the valid FederationDomain
-					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, identityProvider1.ObjectMeta),
 				}
 			},
 			wantActions: func(t *testing.T) []coretesting.Action {
@@ -527,7 +563,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						newCopyWithStatus(invalidFederationDomain,
 							configv1alpha1.FederationDomainPhaseError,
 							[]configv1alpha1.Condition{
-								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider.Name, frozenMetav1Now, 123),
+								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider1.Name, frozenMetav1Now, 123),
 								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
 								sadIssuerURLValidConditionCannotHaveQuery(frozenMetav1Now, 123),
 								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
@@ -538,7 +574,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain2.Namespace,
 						newCopyWithStatus(federationDomain2,
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 				}
@@ -560,12 +596,12 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{Name: "not-duplicate", Namespace: namespace, Generation: 123},
 					Spec:       configv1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/A"}, // different path (paths are case-sensitive)
 				},
-				identityProvider,
+				identityProvider1,
 			},
 			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
 				return []*federationdomainproviders.FederationDomainIssuer{
 					// different path (paths are case-sensitive)
-					federationDomainIssuerWithDefaultIDP(t, "https://issuer-duplicate.com/A", identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, "https://issuer-duplicate.com/A", identityProvider1.ObjectMeta),
 				}
 			},
 			wantActions: func(t *testing.T) []coretesting.Action {
@@ -578,7 +614,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 							},
 							configv1alpha1.FederationDomainPhaseError,
 							[]configv1alpha1.Condition{
-								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider.Name, frozenMetav1Now, 123),
+								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider1.Name, frozenMetav1Now, 123),
 								sadIssuerIsUniqueCondition(frozenMetav1Now, 123),
 								happyIssuerURLValidCondition(frozenMetav1Now, 123),
 								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
@@ -594,7 +630,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 							},
 							configv1alpha1.FederationDomainPhaseError,
 							[]configv1alpha1.Condition{
-								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider.Name, frozenMetav1Now, 123),
+								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider1.Name, frozenMetav1Now, 123),
 								sadIssuerIsUniqueCondition(frozenMetav1Now, 123),
 								happyIssuerURLValidCondition(frozenMetav1Now, 123),
 								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
@@ -609,7 +645,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 								Spec:       configv1alpha1.FederationDomainSpec{Issuer: "https://issuer-duplicate.com/A"},
 							},
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess("https://issuer-duplicate.com/A", identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess("https://issuer-duplicate.com/A", identityProvider1.Name, frozenMetav1Now, 123),
 						),
 					),
 				}
@@ -649,11 +685,11 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						TLS:    &configv1alpha1.FederationDomainTLSSpec{SecretName: "secret1"},
 					},
 				},
-				identityProvider,
+				identityProvider1,
 			},
 			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
 				return []*federationdomainproviders.FederationDomainIssuer{
-					federationDomainIssuerWithDefaultIDP(t, "https://issuer-not-duplicate.com", identityProvider.ObjectMeta),
+					federationDomainIssuerWithDefaultIDP(t, "https://issuer-not-duplicate.com", identityProvider1.ObjectMeta),
 				}
 			},
 			wantActions: func(t *testing.T) []coretesting.Action {
@@ -669,7 +705,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 							},
 							configv1alpha1.FederationDomainPhaseError,
 							[]configv1alpha1.Condition{
-								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider.Name, frozenMetav1Now, 123),
+								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider1.Name, frozenMetav1Now, 123),
 								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
 								happyIssuerURLValidCondition(frozenMetav1Now, 123),
 								sadOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
@@ -688,7 +724,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 							},
 							configv1alpha1.FederationDomainPhaseError,
 							[]configv1alpha1.Condition{
-								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider.Name, frozenMetav1Now, 123),
+								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider1.Name, frozenMetav1Now, 123),
 								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
 								happyIssuerURLValidCondition(frozenMetav1Now, 123),
 								sadOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
@@ -707,7 +743,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 							},
 							configv1alpha1.FederationDomainPhaseError,
 							[]configv1alpha1.Condition{
-								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider.Name, frozenMetav1Now, 123),
+								happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(identityProvider1.Name, frozenMetav1Now, 123),
 								unknownIssuerIsUniqueCondition(frozenMetav1Now, 123),
 								sadIssuerURLValidConditionCannotParse(frozenMetav1Now, 123),
 								unknownOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
@@ -725,28 +761,147 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 								},
 							},
 							configv1alpha1.FederationDomainPhaseReady,
-							allHappyConditionsLegacyConfigurationSuccess("https://issuer-not-duplicate.com", identityProvider.Name, frozenMetav1Now, 123),
+							allHappyConditionsLegacyConfigurationSuccess("https://issuer-not-duplicate.com", identityProvider1.Name, frozenMetav1Now, 123),
+						),
+					),
+				}
+			},
+		},
+		{
+			name: "legacy config: no identity provider specified in federation domain and no identity providers found results in not found status",
+			inputObjects: []runtime.Object{
+				federationDomain1,
+				federationDomain2,
+			},
+			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
+				return []*federationdomainproviders.FederationDomainIssuer{}
+			},
+			wantActions: func(t *testing.T) []coretesting.Action {
+				return []coretesting.Action{
+					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain1.Namespace,
+						newCopyWithStatus(federationDomain1,
+							configv1alpha1.FederationDomainPhaseError,
+							[]configv1alpha1.Condition{
+								sadIdentityProvidersFoundConditionLegacyConfigurationIdentityProviderNotFound(frozenMetav1Now, 123),
+								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
+								happyIssuerURLValidCondition(frozenMetav1Now, 123),
+								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
+								sadReadyCondition(frozenMetav1Now, 123),
+							},
+						),
+					),
+					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain2.Namespace,
+						newCopyWithStatus(federationDomain2,
+							configv1alpha1.FederationDomainPhaseError,
+							[]configv1alpha1.Condition{
+								sadIdentityProvidersFoundConditionLegacyConfigurationIdentityProviderNotFound(frozenMetav1Now, 123),
+								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
+								happyIssuerURLValidCondition(frozenMetav1Now, 123),
+								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
+								sadReadyCondition(frozenMetav1Now, 123),
+							},
+						),
+					),
+				}
+			},
+		},
+		{
+			name: "legacy config: no identity provider specified in federation domain and multiple identity providers found results in not specified status",
+			inputObjects: []runtime.Object{
+				federationDomain1,
+				identityProvider1,
+				identityProvider2,
+			},
+			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
+				return []*federationdomainproviders.FederationDomainIssuer{}
+			},
+			wantActions: func(t *testing.T) []coretesting.Action {
+				return []coretesting.Action{
+					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", federationDomain1.Namespace,
+						newCopyWithStatus(federationDomain1,
+							configv1alpha1.FederationDomainPhaseError,
+							[]configv1alpha1.Condition{
+								sadIdentityProvidersFoundConditionIdentityProviderNotSpecified(2, frozenMetav1Now, 123),
+								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
+								happyIssuerURLValidCondition(frozenMetav1Now, 123),
+								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
+								sadReadyCondition(frozenMetav1Now, 123),
+							},
+						),
+					),
+				}
+			},
+		},
+		{
+			name: "the federation domain specifies identity providers that cannot be found",
+			inputObjects: []runtime.Object{
+				&configv1alpha1.FederationDomain{
+					ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
+					Spec: configv1alpha1.FederationDomainSpec{
+						Issuer: "https://issuer1.com",
+						IdentityProviders: []configv1alpha1.FederationDomainIdentityProvider{
+							{
+								DisplayName: "cant-find-me",
+								ObjectRef: corev1.TypedLocalObjectReference{
+									APIGroup: pointer.String(apiGroupSupervisor),
+									Kind:     "OIDCIdentityProvider",
+									Name:     "cant-find-me",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantFederationDomainIssuers: func(t *testing.T) []*federationdomainproviders.FederationDomainIssuer {
+				return []*federationdomainproviders.FederationDomainIssuer{}
+			},
+			// TODO: sketch out a way to eliminate the whole wantActions and be more precise
+			// if not wantActions, might wantStatusUpdate?
+			// wantStatusUpdate: func(t *testing.T, []configv1alpha1.FederationDomain) {
+			// 	{
+			// 		Name: "foo",
+			// 		Namespace: "bar",
+			// 		Phase: "Happy",
+			// 		Conditions: []metav1.Condition{},
+			// 	}
+			// },
+			wantActions: func(t *testing.T) []coretesting.Action {
+				return []coretesting.Action{
+					coretesting.NewUpdateSubresourceAction(federationDomainGVR, "status", namespace,
+						newCopyWithStatus(
+							&configv1alpha1.FederationDomain{
+								ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
+								Spec: configv1alpha1.FederationDomainSpec{
+									Issuer: "https://issuer1.com",
+									IdentityProviders: []configv1alpha1.FederationDomainIdentityProvider{
+										{
+											DisplayName: "cant-find-me",
+											ObjectRef: corev1.TypedLocalObjectReference{
+												APIGroup: pointer.String(apiGroupSupervisor),
+												Kind:     "OIDCIdentityProvider",
+												Name:     "cant-find-me",
+											},
+										},
+									},
+								},
+							},
+							configv1alpha1.FederationDomainPhaseError,
+							[]configv1alpha1.Condition{
+								sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(
+									`.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: `+
+										`IDP with displayName "cant-find-me" at index 0`, frozenMetav1Now, 123),
+								happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
+								happyIssuerURLValidCondition(frozenMetav1Now, 123),
+								happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
+								sadReadyCondition(frozenMetav1Now, 123),
+							},
 						),
 					),
 				}
 			},
 		},
-		// TODO(Ben): add these additional tests to cover the new cases. There will likely also be more as we cover
-		//    both the truthy as well as the falsy cases.
 		// {
-		// 	name:    "legacy config: no identity provider specified in federation domain and no identity providers found",
+		// 	name:    "the federation domain specifies identity providers that exist",
-		// 	wantErr: "...please create an identity provider resource",
-		// },
-		// {
-		// 	name:    "legacy config: no identity provider specified in federation domain and multiple identity providers found",
-		// 	wantErr: "...to specify which identity providers this federation domain should use",
-		// },
-		// {
-		// 	name:    "the federation domain specifies identity providers that cannot be found", // single and/or multiple?
-		// 	wantErr: "...identifies resource(s) that cannot be found: {list.of...}",
-		// },
-		// {
-		// 	name:    "the federation domain specifies identity providers taht exist",
 		// 	wantErr: "", // n/a
 		// },
 	}