Validate IDP objectRef kind names in federation_domain_watcher.go

Co-authored-by: Benjamin A. Petersen <ben@benjaminapetersen.me>

internal/controller/supervisorconfig/federation_domain_watcher.go

@@ -35,13 +35,14 @@ import (
 )
 
 const (
-	typeReady                         = "Ready"
+	typeReady                                = "Ready"
-	typeIssuerURLValid                = "IssuerURLValid"
+	typeIssuerURLValid                       = "IssuerURLValid"
-	typeOneTLSSecretPerIssuerHostname = "OneTLSSecretPerIssuerHostname"
+	typeOneTLSSecretPerIssuerHostname        = "OneTLSSecretPerIssuerHostname"
-	typeIssuerIsUnique                = "IssuerIsUnique"
+	typeIssuerIsUnique                       = "IssuerIsUnique"
-	typeIdentityProvidersFound        = "IdentityProvidersFound"
+	typeIdentityProvidersFound               = "IdentityProvidersFound"
-	typeDisplayNamesUnique            = "DisplayNamesUnique"
+	typeIdentityProvidersDisplayNamesUnique  = "IdentityProvidersDisplayNamesUnique"
-	typeAPIGroupSuffixValid           = "APIGroupSuffixValid"
+	typeIdentityProvidersAPIGroupSuffixValid = "IdentityProvidersObjectRefAPIGroupSuffixValid"
+	typeIdentityProvidersObjectRefKindValid  = "IdentityProvidersObjectRefKindValid"
 
 	reasonSuccess                                     = "Success"
 	reasonNotReady                                    = "NotReady"
@@ -54,7 +55,12 @@ const (
 	reasonIdentityProvidersObjectRefsNotFound         = "IdentityProvidersObjectRefsNotFound"
 	reasonIdentityProviderNotSpecified                = "IdentityProviderNotSpecified"
 	reasonDuplicateDisplayNames                       = "DuplicateDisplayNames"
-	reasonAPIGroupNameUnrecognized                    = "APIGroupNameUnrecognized"
+	reasonAPIGroupNameUnrecognized                    = "APIGroupUnrecognized"
+	reasonKindUnrecognized                            = "KindUnrecognized"
+
+	kindLDAPIdentityProvider            = "LDAPIdentityProvider"
+	kindOIDCIdentityProvider            = "OIDCIdentityProvider"
+	kindActiveDirectoryIdentityProvider = "ActiveDirectoryIdentityProvider"
 
 	celTransformerMaxExpressionRuntime = 5 * time.Second
 )
@@ -78,6 +84,7 @@ type federationDomainWatcherController struct {
 	activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer
 
 	celTransformer *celtransformer.CELTransformer
+	allowedKinds   sets.Set[string]
 }
 
 // NewFederationDomainWatcherController creates a controllerlib.Controller that watches
@@ -93,6 +100,7 @@ func NewFederationDomainWatcherController(
 	activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer,
 	withInformer pinnipedcontroller.WithInformerOptionFunc,
 ) controllerlib.Controller {
+	allowedKinds := sets.New(kindActiveDirectoryIdentityProvider, kindLDAPIdentityProvider, kindOIDCIdentityProvider)
 	return controllerlib.New(
 		controllerlib.Config{
 			Name: "FederationDomainWatcherController",
@@ -105,6 +113,7 @@ func NewFederationDomainWatcherController(
 				oidcIdentityProviderInformer:            oidcIdentityProviderInformer,
 				ldapIdentityProviderInformer:            ldapIdentityProviderInformer,
 				activeDirectoryIdentityProviderInformer: activeDirectoryIdentityProviderInformer,
+				allowedKinds:                            allowedKinds,
 			},
 		},
 		withInformer(
@@ -306,8 +315,9 @@ func (c *federationDomainWatcherController) makeLegacyFederationDomainIssuer(
 	federationDomainIssuer, err := federationdomainproviders.NewFederationDomainIssuerWithDefaultIDP(federationDomain.Spec.Issuer, defaultFederationDomainIdentityProvider)
 	conditions = appendIssuerURLValidCondition(err, conditions)
 
-	conditions = appendDuplicateDisplayNamesCondition(sets.Set[string]{}, conditions)
+	conditions = appendIdentityProviderDuplicateDisplayNamesCondition(sets.Set[string]{}, conditions)
-	conditions = appendAPIGroupSuffixCondition(c.apiGroup, sets.Set[string]{}, conditions)
+	conditions = appendIdentityProviderObjectRefAPIGroupSuffixCondition(c.apiGroup, []string{}, conditions)
+	conditions = appendIdentityProviderObjectRefKindCondition(c.sortedAllowedKinds(), []string{}, conditions)
 
 	return federationDomainIssuer, conditions, nil
 }
@@ -320,30 +330,47 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
 	idpNotFoundIndices := []int{}
 	displayNames := sets.Set[string]{}
 	duplicateDisplayNames := sets.Set[string]{}
-	badAPIGroupNames := sets.Set[string]{}
+	badAPIGroupNames := []string{}
+	badKinds := []string{}
 
 	for index, idp := range federationDomain.Spec.IdentityProviders {
+		// The CRD requires the displayName field, and validates that it has at least one character,
+		// so here we only need to validate that they are unique.
 		if displayNames.Has(idp.DisplayName) {
 			duplicateDisplayNames.Insert(idp.DisplayName)
 		}
 		displayNames.Insert(idp.DisplayName)
 
-		apiGroup := "nil"
+		// The objectRef is a required field in the CRD, so it will always exist in practice.
+		// objectRef.name and objectRef.kind are required, but may be empty strings.
+		// objectRef.apiGroup is not required, however, so it may be nil or empty string.
+		canTryToFindIDP := true
+		apiGroup := ""
 		if idp.ObjectRef.APIGroup != nil {
 			apiGroup = *idp.ObjectRef.APIGroup
 		}
 		if apiGroup != c.apiGroup {
-			badAPIGroupNames.Insert(apiGroup)
+			badAPIGroupNames = append(badAPIGroupNames, apiGroup)
+			canTryToFindIDP = false
+		}
+		if !c.allowedKinds.Has(idp.ObjectRef.Kind) {
+			badKinds = append(badKinds, idp.ObjectRef.Kind)
+			canTryToFindIDP = false
 		}
 
-		// Validate that each objectRef resolves to an existing IDP. It does not matter if the IDP itself
+		var idpResourceUID types.UID
-		// is phase=Ready, because it will not be loaded into the cache if not ready. For each objectRef
+		idpWasFound := false
-		// that does not resolve, put an error on the FederationDomain status.
+		if canTryToFindIDP {
-		idpResourceUID, idpWasFound, err := c.findIDPsUIDByObjectRef(idp.ObjectRef, federationDomain.Namespace)
+			var err error
-		if err != nil {
+			// Validate that each objectRef resolves to an existing IDP. It does not matter if the IDP itself
-			return nil, nil, err
+			// is phase=Ready, because it will not be loaded into the cache if not ready. For each objectRef
+			// that does not resolve, put an error on the FederationDomain status.
+			idpResourceUID, idpWasFound, err = c.findIDPsUIDByObjectRef(idp.ObjectRef, federationDomain.Namespace)
+			if err != nil {
+				return nil, nil, err
+			}
 		}
-		if !idpWasFound {
+		if !canTryToFindIDP || !idpWasFound {
 			idpNotFoundIndices = append(idpNotFoundIndices, index)
 		}
 
@@ -391,26 +418,27 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
 	federationDomainIssuer, err := federationdomainproviders.NewFederationDomainIssuer(federationDomain.Spec.Issuer, federationDomainIdentityProviders)
 	conditions = appendIssuerURLValidCondition(err, conditions)
 
-	conditions = appendDuplicateDisplayNamesCondition(duplicateDisplayNames, conditions)
+	conditions = appendIdentityProviderDuplicateDisplayNamesCondition(duplicateDisplayNames, conditions)
-	conditions = appendAPIGroupSuffixCondition(c.apiGroup, badAPIGroupNames, conditions)
+	conditions = appendIdentityProviderObjectRefAPIGroupSuffixCondition(c.apiGroup, badAPIGroupNames, conditions)
+	conditions = appendIdentityProviderObjectRefKindCondition(c.sortedAllowedKinds(), badKinds, conditions)
 
 	return federationDomainIssuer, conditions, nil
 }
-
 func (c *federationDomainWatcherController) findIDPsUIDByObjectRef(objectRef corev1.TypedLocalObjectReference, namespace string) (types.UID, bool, error) {
 	var idpResourceUID types.UID
 	var foundIDP metav1.Object
 	var err error
 
 	switch objectRef.Kind {
-	case "LDAPIdentityProvider":
+	case kindLDAPIdentityProvider:
 		foundIDP, err = c.ldapIdentityProviderInformer.Lister().LDAPIdentityProviders(namespace).Get(objectRef.Name)
-	case "ActiveDirectoryIdentityProvider":
+	case kindActiveDirectoryIdentityProvider:
 		foundIDP, err = c.activeDirectoryIdentityProviderInformer.Lister().ActiveDirectoryIdentityProviders(namespace).Get(objectRef.Name)
-	case "OIDCIdentityProvider":
+	case kindOIDCIdentityProvider:
 		foundIDP, err = c.oidcIdentityProviderInformer.Lister().OIDCIdentityProviders(namespace).Get(objectRef.Name)
 	default:
-		// TODO: handle an IDP type that we do not understand by writing a status condition
+		// This shouldn't happen because this helper function is not called when the kind is invalid.
+		return "", false, fmt.Errorf("unexpected kind: %s", objectRef.Kind)
 	}
 
 	switch {
@@ -419,7 +447,7 @@ func (c *federationDomainWatcherController) findIDPsUIDByObjectRef(objectRef cor
 	case errors.IsNotFound(err):
 		return "", false, nil
 	default:
-		return "", false, err // unexpected error
+		return "", false, err // unexpected error from the informer
 	}
 	return idpResourceUID, true, nil
 }
@@ -555,20 +583,42 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
 	return pipeline, nil
 }
 
-func appendAPIGroupSuffixCondition(expectedSuffixName string, badSuffixNames sets.Set[string], conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
+func (c *federationDomainWatcherController) sortedAllowedKinds() []string {
-	if badSuffixNames.Len() > 0 {
+	return sortAndQuote(c.allowedKinds.UnsortedList())
-		badNames := badSuffixNames.UnsortedList()
+}
-		sort.Strings(badNames)
+
+func appendIdentityProviderObjectRefKindCondition(expectedKinds []string, badSuffixNames []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
+	if len(badSuffixNames) > 0 {
 		conditions = append(conditions, &configv1alpha1.Condition{
-			Type:   typeAPIGroupSuffixValid,
+			Type:   typeIdentityProvidersObjectRefKindValid,
 			Status: configv1alpha1.ConditionFalse,
-			Reason: reasonAPIGroupNameUnrecognized,
+			Reason: reasonKindUnrecognized,
-			Message: fmt.Sprintf("the API groups specified by .spec.identityProviders[].objectRef.apiGroup are not recognized (should be %q): %s",
+			Message: fmt.Sprintf("the kinds specified by .spec.identityProviders[].objectRef.kind are not recognized (should be one of %s): %s",
-				expectedSuffixName, strings.Join(badNames, ", ")),
+				strings.Join(expectedKinds, ", "), strings.Join(sortAndQuote(badSuffixNames), ", ")),
 		})
 	} else {
 		conditions = append(conditions, &configv1alpha1.Condition{
-			Type:    typeAPIGroupSuffixValid,
+			Type:    typeIdentityProvidersObjectRefKindValid,
+			Status:  configv1alpha1.ConditionTrue,
+			Reason:  reasonSuccess,
+			Message: "the kinds specified by .spec.identityProviders[].objectRef.kind are recognized",
+		})
+	}
+	return conditions
+}
+
+func appendIdentityProviderObjectRefAPIGroupSuffixCondition(expectedSuffixName string, badSuffixNames []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
+	if len(badSuffixNames) > 0 {
+		conditions = append(conditions, &configv1alpha1.Condition{
+			Type:   typeIdentityProvidersAPIGroupSuffixValid,
+			Status: configv1alpha1.ConditionFalse,
+			Reason: reasonAPIGroupNameUnrecognized,
+			Message: fmt.Sprintf("the API groups specified by .spec.identityProviders[].objectRef.apiGroup are not recognized (should be %q): %s",
+				expectedSuffixName, strings.Join(sortAndQuote(badSuffixNames), ", ")),
+		})
+	} else {
+		conditions = append(conditions, &configv1alpha1.Condition{
+			Type:    typeIdentityProvidersAPIGroupSuffixValid,
 			Status:  configv1alpha1.ConditionTrue,
 			Reason:  reasonSuccess,
 			Message: "the API groups specified by .spec.identityProviders[].objectRef.apiGroup are recognized",
@@ -577,20 +627,18 @@ func appendAPIGroupSuffixCondition(expectedSuffixName string, badSuffixNames set
 	return conditions
 }
 
-func appendDuplicateDisplayNamesCondition(duplicateDisplayNames sets.Set[string], conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
+func appendIdentityProviderDuplicateDisplayNamesCondition(duplicateDisplayNames sets.Set[string], conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
 	if duplicateDisplayNames.Len() > 0 {
-		duplicates := duplicateDisplayNames.UnsortedList()
-		sort.Strings(duplicates)
 		conditions = append(conditions, &configv1alpha1.Condition{
-			Type:   typeDisplayNamesUnique,
+			Type:   typeIdentityProvidersDisplayNamesUnique,
 			Status: configv1alpha1.ConditionFalse,
 			Reason: reasonDuplicateDisplayNames,
 			Message: fmt.Sprintf("the names specified by .spec.identityProviders[].displayName contain duplicates: %s",
-				strings.Join(duplicates, ", ")),
+				strings.Join(sortAndQuote(duplicateDisplayNames.UnsortedList()), ", ")),
 		})
 	} else {
 		conditions = append(conditions, &configv1alpha1.Condition{
-			Type:    typeDisplayNamesUnique,
+			Type:    typeIdentityProvidersDisplayNamesUnique,
 			Status:  configv1alpha1.ConditionTrue,
 			Reason:  reasonSuccess,
 			Message: "the names specified by .spec.identityProviders[].displayName are unique",
@@ -620,6 +668,15 @@ func appendIssuerURLValidCondition(err error, conditions []*configv1alpha1.Condi
 	return conditions
 }
 
+func sortAndQuote(strs []string) []string {
+	quoted := make([]string, 0, len(strs))
+	for _, s := range strs {
+		quoted = append(quoted, fmt.Sprintf("%q", s))
+	}
+	sort.Strings(quoted)
+	return quoted
+}
+
 func (c *federationDomainWatcherController) updateStatus(
 	ctx context.Context,
 	federationDomain *configv1alpha1.FederationDomain,

internal/controller/supervisorconfig/federation_domain_watcher_test.go

@@ -364,20 +364,20 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 		}
 	}
 
-	sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound := func(msg string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
+	sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound := func(idpsNotFound string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
 		return configv1alpha1.Condition{
 			Type:               "IdentityProvidersFound",
 			Status:             "False",
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
 			Reason:             "IdentityProvidersObjectRefsNotFound",
-			Message:            msg,
+			Message:            fmt.Sprintf(".spec.identityProviders[].objectRef identifies resource(s) that cannot be found: %s", idpsNotFound),
 		}
 	}
 
 	happyDisplayNamesUniqueCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
 		return configv1alpha1.Condition{
-			Type:               "DisplayNamesUnique",
+			Type:               "IdentityProvidersDisplayNamesUnique",
 			Status:             "True",
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
@@ -388,7 +388,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 
 	sadDisplayNamesUniqueCondition := func(duplicateNames string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
 		return configv1alpha1.Condition{
-			Type:               "DisplayNamesUnique",
+			Type:               "IdentityProvidersDisplayNamesUnique",
 			Status:             "False",
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
@@ -399,7 +399,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 
 	happyAPIGroupSuffixCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
 		return configv1alpha1.Condition{
-			Type:               "APIGroupSuffixValid",
+			Type:               "IdentityProvidersObjectRefAPIGroupSuffixValid",
 			Status:             "True",
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
@@ -408,20 +408,53 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 		}
 	}
 
-	sadAPIGroupSuffixCondition := func(badNames string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
+	sadAPIGroupSuffixCondition := func(badApiGroups string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
 		return configv1alpha1.Condition{
-			Type:               "APIGroupSuffixValid",
+			Type:               "IdentityProvidersObjectRefAPIGroupSuffixValid",
 			Status:             "False",
 			ObservedGeneration: observedGeneration,
 			LastTransitionTime: time,
-			Reason:             "APIGroupNameUnrecognized",
+			Reason:             "APIGroupUnrecognized",
-			Message:            fmt.Sprintf("the API groups specified by .spec.identityProviders[].objectRef.apiGroup are not recognized (should be \"idp.supervisor.%s\"): %s", apiGroupSuffix, badNames),
+			Message: fmt.Sprintf("the API groups specified by .spec.identityProviders[].objectRef.apiGroup "+
+				"are not recognized (should be \"idp.supervisor.%s\"): %s", apiGroupSuffix, badApiGroups),
 		}
 	}
 
+	happyKindCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
+		return configv1alpha1.Condition{
+			Type:               "IdentityProvidersObjectRefKindValid",
+			Status:             "True",
+			ObservedGeneration: observedGeneration,
+			LastTransitionTime: time,
+			Reason:             "Success",
+			Message:            "the kinds specified by .spec.identityProviders[].objectRef.kind are recognized",
+		}
+	}
+
+	sadKindCondition := func(badKinds string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
+		return configv1alpha1.Condition{
+			Type:               "IdentityProvidersObjectRefKindValid",
+			Status:             "False",
+			ObservedGeneration: observedGeneration,
+			LastTransitionTime: time,
+			Reason:             "KindUnrecognized",
+			Message: fmt.Sprintf(`the kinds specified by .spec.identityProviders[].objectRef.kind are `+
+				`not recognized (should be one of "ActiveDirectoryIdentityProvider", "LDAPIdentityProvider", "OIDCIdentityProvider"): %s`, badKinds),
+		}
+	}
+
+	sortConditionsByType := func(c []configv1alpha1.Condition) []configv1alpha1.Condition {
+		cp := make([]configv1alpha1.Condition, len(c))
+		copy(cp, c)
+		sort.SliceStable(cp, func(i, j int) bool {
+			return cp[i].Type < cp[j].Type
+		})
+		return cp
+	}
+
 	allHappyConditionsLegacyConfigurationSuccess := func(issuer string, idpName string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition {
-		return []configv1alpha1.Condition{
+		return sortConditionsByType([]configv1alpha1.Condition{
-			// expect them to be sorted alphabetically by type
+			happyKindCondition(frozenMetav1Now, 123),
 			happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 			happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 			happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(idpName, time, observedGeneration),
@@ -429,12 +462,12 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			happyIssuerURLValidCondition(time, observedGeneration),
 			happyOneTLSSecretPerIssuerHostnameCondition(time, observedGeneration),
 			happyReadyCondition(issuer, time, observedGeneration),
-		}
+		})
 	}
 
 	allHappyConditionsSuccess := func(issuer string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition {
-		return []configv1alpha1.Condition{
+		return sortConditionsByType([]configv1alpha1.Condition{
-			// expect them to be sorted alphabetically by type
+			happyKindCondition(frozenMetav1Now, 123),
 			happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 			happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 			happyIdentityProvidersFoundConditionSuccess(frozenMetav1Now, 123),
@@ -442,7 +475,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			happyIssuerURLValidCondition(frozenMetav1Now, 123),
 			happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 			happyReadyCondition(issuer, frozenMetav1Now, 123),
-		}
+		})
 	}
 
 	invalidIssuerURL := ":/host//path"
@@ -564,7 +597,8 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			wantStatusUpdates: []*configv1alpha1.FederationDomain{
 				expectedFederationDomainStatusUpdate(invalidIssuerURLFederationDomain,
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(oidcIdentityProvider.Name, frozenMetav1Now, 123),
@@ -572,7 +606,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						sadIssuerURLValidConditionCannotHaveQuery(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(federationDomain2,
 					configv1alpha1.FederationDomainPhaseReady,
@@ -609,7 +643,8 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			wantStatusUpdates: []*configv1alpha1.FederationDomain{
 				expectedFederationDomainStatusUpdate(invalidIssuerURLFederationDomain,
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(oidcIdentityProvider.Name, frozenMetav1Now, 123),
@@ -617,7 +652,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						sadIssuerURLValidConditionCannotHaveQuery(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(federationDomain2,
 					configv1alpha1.FederationDomainPhaseReady,
@@ -652,7 +687,8 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						ObjectMeta: metav1.ObjectMeta{Name: "duplicate1", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(oidcIdentityProvider.Name, frozenMetav1Now, 123),
@@ -660,14 +696,15 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(
 					&configv1alpha1.FederationDomain{
 						ObjectMeta: metav1.ObjectMeta{Name: "duplicate2", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(oidcIdentityProvider.Name, frozenMetav1Now, 123),
@@ -675,7 +712,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(
 					&configv1alpha1.FederationDomain{
@@ -731,7 +768,8 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						ObjectMeta: metav1.ObjectMeta{Name: "fd1", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(oidcIdentityProvider.Name, frozenMetav1Now, 123),
@@ -739,14 +777,15 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						sadOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(
 					&configv1alpha1.FederationDomain{
 						ObjectMeta: metav1.ObjectMeta{Name: "fd2", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(oidcIdentityProvider.Name, frozenMetav1Now, 123),
@@ -754,14 +793,15 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						sadOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(
 					&configv1alpha1.FederationDomain{
 						ObjectMeta: metav1.ObjectMeta{Name: "invalidIssuerURLFederationDomain", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(oidcIdentityProvider.Name, frozenMetav1Now, 123),
@@ -769,7 +809,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						sadIssuerURLValidConditionCannotParse(frozenMetav1Now, 123),
 						unknownOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(
 					&configv1alpha1.FederationDomain{
@@ -790,7 +830,8 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			wantStatusUpdates: []*configv1alpha1.FederationDomain{
 				expectedFederationDomainStatusUpdate(federationDomain1,
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						sadIdentityProvidersFoundConditionLegacyConfigurationIdentityProviderNotFound(frozenMetav1Now, 123),
@@ -798,11 +839,12 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 				expectedFederationDomainStatusUpdate(federationDomain2,
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						sadIdentityProvidersFoundConditionLegacyConfigurationIdentityProviderNotFound(frozenMetav1Now, 123),
@@ -810,7 +852,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 			},
 		},
@@ -826,7 +868,8 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 			wantStatusUpdates: []*configv1alpha1.FederationDomain{
 				expectedFederationDomainStatusUpdate(federationDomain1,
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						sadIdentityProvidersFoundConditionIdentityProviderNotSpecified(3, frozenMetav1Now, 123),
@@ -834,7 +877,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 			},
 		},
@@ -881,12 +924,12 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
 						sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(
-							`.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: `+
+							`.spec.identityProviders[0] with displayName "cant-find-me", `+
-								`.spec.identityProviders[0] with displayName "cant-find-me", `+
 								`.spec.identityProviders[1] with displayName "cant-find-me-either", `+
 								`.spec.identityProviders[2] with displayName "cant-find-me-still"`,
 							frozenMetav1Now, 123),
@@ -894,7 +937,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					},
+					}),
 				),
 			},
 		},
@@ -1029,15 +1072,16 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
+						happyKindCondition(frozenMetav1Now, 123),
 						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
-						sadDisplayNamesUniqueCondition("duplicate1, duplicate2", frozenMetav1Now, 123),
+						sadDisplayNamesUniqueCondition(`"duplicate1", "duplicate2"`, frozenMetav1Now, 123),
 						happyIdentityProvidersFoundConditionSuccess(frozenMetav1Now, 123),
 						happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					}),
+					})),
 			},
 		},
 		{
@@ -1062,7 +1106,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 							{
 								DisplayName: "name2",
 								ObjectRef: corev1.TypedLocalObjectReference{
-									APIGroup: pointer.String("also-wrong.example.com"),
+									APIGroup: pointer.String(""), // empty string is wrong
 									Kind:     "LDAPIdentityProvider",
 									Name:     ldapIdentityProvider.Name,
 								},
@@ -1070,7 +1114,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 							{
 								DisplayName: "name3",
 								ObjectRef: corev1.TypedLocalObjectReference{
-									APIGroup: nil, // also wrong
+									APIGroup: nil, // nil is wrong, and gets treated like an empty string in the error condition
 									Kind:     "LDAPIdentityProvider",
 									Name:     ldapIdentityProvider.Name,
 								},
@@ -1094,15 +1138,81 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
 						ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
 					},
 					configv1alpha1.FederationDomainPhaseError,
-					[]configv1alpha1.Condition{
+					sortConditionsByType([]configv1alpha1.Condition{
-						sadAPIGroupSuffixCondition("also-wrong.example.com, nil, wrong.example.com", frozenMetav1Now, 123),
+						happyKindCondition(frozenMetav1Now, 123),
+						sadAPIGroupSuffixCondition(`"", "", "wrong.example.com"`, frozenMetav1Now, 123),
 						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
-						happyIdentityProvidersFoundConditionSuccess(frozenMetav1Now, 123),
+						sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(
+							`.spec.identityProviders[0] with displayName "name1", `+
+								`.spec.identityProviders[1] with displayName "name2", `+
+								`.spec.identityProviders[2] with displayName "name3"`,
+							frozenMetav1Now, 123),
 						happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
 						happyIssuerURLValidCondition(frozenMetav1Now, 123),
 						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
 						sadReadyCondition(frozenMetav1Now, 123),
-					}),
+					})),
+			},
+		},
+		{
+			name: "the federation domain has unrecognized kind names in objectRefs",
+			inputObjects: []runtime.Object{
+				oidcIdentityProvider,
+				ldapIdentityProvider,
+				adIdentityProvider,
+				&configv1alpha1.FederationDomain{
+					ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
+					Spec: configv1alpha1.FederationDomainSpec{
+						Issuer: "https://issuer1.com",
+						IdentityProviders: []configv1alpha1.FederationDomainIdentityProvider{
+							{
+								DisplayName: "name1",
+								ObjectRef: corev1.TypedLocalObjectReference{
+									APIGroup: pointer.String(apiGroupSupervisor),
+									Kind:     "OIDCIdentityProvider", // correct
+									Name:     oidcIdentityProvider.Name,
+								},
+							},
+							{
+								DisplayName: "name2",
+								ObjectRef: corev1.TypedLocalObjectReference{
+									APIGroup: pointer.String(apiGroupSupervisor),
+									Kind:     "wrong",
+									Name:     ldapIdentityProvider.Name,
+								},
+							},
+							{
+								DisplayName: "name3",
+								ObjectRef: corev1.TypedLocalObjectReference{
+									APIGroup: pointer.String(apiGroupSupervisor),
+									Kind:     "", // empty is also wrong
+									Name:     ldapIdentityProvider.Name,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{},
+			wantStatusUpdates: []*configv1alpha1.FederationDomain{
+				expectedFederationDomainStatusUpdate(
+					&configv1alpha1.FederationDomain{
+						ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
+					},
+					configv1alpha1.FederationDomainPhaseError,
+					sortConditionsByType([]configv1alpha1.Condition{
+						sadKindCondition(`"", "wrong"`, frozenMetav1Now, 123),
+						happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
+						happyDisplayNamesUniqueCondition(frozenMetav1Now, 123),
+						sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(
+							`.spec.identityProviders[1] with displayName "name2", `+
+								`.spec.identityProviders[2] with displayName "name3"`,
+							frozenMetav1Now, 123),
+						happyIssuerIsUniqueCondition(frozenMetav1Now, 123),
+						happyIssuerURLValidCondition(frozenMetav1Now, 123),
+						happyOneTLSSecretPerIssuerHostnameCondition(frozenMetav1Now, 123),
+						sadReadyCondition(frozenMetav1Now, 123),
+					})),
 			},
 		},
 		{

test/integration/supervisor_discovery_test.go

@@ -133,18 +133,24 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) {
 			config6Duplicate1, _ := requireCreatingFederationDomainCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer6, client)
 			config6Duplicate2 := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{Issuer: issuer6}, v1alpha1.FederationDomainPhaseError)
 			requireStatus(t, client, ns, config6Duplicate1.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{
-				"Ready":                         v1alpha1.ConditionFalse,
+				"Ready":                                         v1alpha1.ConditionFalse,
-				"IssuerIsUnique":                v1alpha1.ConditionFalse,
+				"IssuerIsUnique":                                v1alpha1.ConditionFalse,
-				"IdentityProvidersFound":        v1alpha1.ConditionTrue,
+				"IdentityProvidersFound":                        v1alpha1.ConditionTrue,
-				"OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue,
+				"OneTLSSecretPerIssuerHostname":                 v1alpha1.ConditionTrue,
-				"IssuerURLValid":                v1alpha1.ConditionTrue,
+				"IssuerURLValid":                                v1alpha1.ConditionTrue,
+				"IdentityProvidersObjectRefKindValid":           v1alpha1.ConditionTrue,
+				"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
+				"IdentityProvidersDisplayNamesUnique":           v1alpha1.ConditionTrue,
 			})
 			requireStatus(t, client, ns, config6Duplicate2.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{
-				"Ready":                         v1alpha1.ConditionFalse,
+				"Ready":                                         v1alpha1.ConditionFalse,
-				"IssuerIsUnique":                v1alpha1.ConditionFalse,
+				"IssuerIsUnique":                                v1alpha1.ConditionFalse,
-				"IdentityProvidersFound":        v1alpha1.ConditionTrue,
+				"IdentityProvidersFound":                        v1alpha1.ConditionTrue,
-				"OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue,
+				"OneTLSSecretPerIssuerHostname":                 v1alpha1.ConditionTrue,
-				"IssuerURLValid":                v1alpha1.ConditionTrue,
+				"IssuerURLValid":                                v1alpha1.ConditionTrue,
+				"IdentityProvidersObjectRefKindValid":           v1alpha1.ConditionTrue,
+				"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
+				"IdentityProvidersDisplayNamesUnique":           v1alpha1.ConditionTrue,
 			})
 			requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, issuer6)
 
@@ -164,11 +170,14 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) {
 			// When we create a provider with an invalid issuer, the status is set to invalid.
 			badConfig := testlib.CreateTestFederationDomain(ctx, t, v1alpha1.FederationDomainSpec{Issuer: badIssuer}, v1alpha1.FederationDomainPhaseError)
 			requireStatus(t, client, ns, badConfig.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{
-				"Ready":                         v1alpha1.ConditionFalse,
+				"Ready":                                         v1alpha1.ConditionFalse,
-				"IssuerIsUnique":                v1alpha1.ConditionTrue,
+				"IssuerIsUnique":                                v1alpha1.ConditionTrue,
-				"IdentityProvidersFound":        v1alpha1.ConditionTrue,
+				"IdentityProvidersFound":                        v1alpha1.ConditionTrue,
-				"OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue,
+				"OneTLSSecretPerIssuerHostname":                 v1alpha1.ConditionTrue,
-				"IssuerURLValid":                v1alpha1.ConditionFalse,
+				"IssuerURLValid":                                v1alpha1.ConditionFalse,
+				"IdentityProvidersObjectRefKindValid":           v1alpha1.ConditionTrue,
+				"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
+				"IdentityProvidersDisplayNamesUnique":           v1alpha1.ConditionTrue,
 			})
 			requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, badIssuer)
 			requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, badConfig, client, ns, scheme, addr, caBundle, badIssuer)
@@ -677,11 +686,14 @@ func requireDelete(t *testing.T, client pinnipedclientset.Interface, ns, name st
 
 func requireFullySuccessfulStatus(t *testing.T, client pinnipedclientset.Interface, ns, name string) {
 	requireStatus(t, client, ns, name, v1alpha1.FederationDomainPhaseReady, map[string]v1alpha1.ConditionStatus{
-		"Ready":                         v1alpha1.ConditionTrue,
+		"Ready":                                         v1alpha1.ConditionTrue,
-		"IssuerIsUnique":                v1alpha1.ConditionTrue,
+		"IssuerIsUnique":                                v1alpha1.ConditionTrue,
-		"IdentityProvidersFound":        v1alpha1.ConditionTrue,
+		"IdentityProvidersFound":                        v1alpha1.ConditionTrue,
-		"OneTLSSecretPerIssuerHostname": v1alpha1.ConditionTrue,
+		"OneTLSSecretPerIssuerHostname":                 v1alpha1.ConditionTrue,
-		"IssuerURLValid":                v1alpha1.ConditionTrue,
+		"IssuerURLValid":                                v1alpha1.ConditionTrue,
+		"IdentityProvidersObjectRefKindValid":           v1alpha1.ConditionTrue,
+		"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
+		"IdentityProvidersDisplayNamesUnique":           v1alpha1.ConditionTrue,
 	})
 }