diff --git a/internal/controller/supervisorconfig/federation_domain_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go index 98f91fb72..6c467e923 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -664,7 +664,7 @@ func appendIdentityProviderObjectRefKindCondition(expectedKinds []string, badSuf Type: typeIdentityProvidersObjectRefKindValid, Status: configv1alpha1.ConditionFalse, Reason: reasonKindUnrecognized, - Message: fmt.Sprintf("the kinds specified by .spec.identityProviders[].objectRef.kind are not recognized (should be one of %s): %s", + Message: fmt.Sprintf("some kinds specified by .spec.identityProviders[].objectRef.kind are not recognized (should be one of %s): %s", strings.Join(expectedKinds, ", "), strings.Join(sortAndQuote(badSuffixNames), ", ")), }) } else { @@ -684,17 +684,16 @@ func appendIdentityProvidersFoundCondition( conditions []*configv1alpha1.Condition, ) []*configv1alpha1.Condition { if len(idpNotFoundIndices) != 0 { - msgs := []string{} + messages := []string{} for _, idpNotFoundIndex := range idpNotFoundIndices { - msgs = append(msgs, fmt.Sprintf(".spec.identityProviders[%d] with displayName %q", idpNotFoundIndex, - federationDomainIdentityProviders[idpNotFoundIndex].DisplayName)) + messages = append(messages, fmt.Sprintf("cannot find resource specified by .spec.identityProviders[%d].objectRef (with name %q)", + idpNotFoundIndex, federationDomainIdentityProviders[idpNotFoundIndex].ObjectRef.Name)) } conditions = append(conditions, &configv1alpha1.Condition{ - Type: typeIdentityProvidersFound, - Status: configv1alpha1.ConditionFalse, - Reason: reasonIdentityProvidersObjectRefsNotFound, - Message: fmt.Sprintf(".spec.identityProviders[].objectRef identifies resource(s) that cannot be found: %s", - strings.Join(msgs, ", ")), + Type: typeIdentityProvidersFound, + Status: configv1alpha1.ConditionFalse, + Reason: reasonIdentityProvidersObjectRefsNotFound, + Message: strings.Join(messages, "\n\n"), }) } else if len(federationDomainIdentityProviders) != 0 { conditions = append(conditions, &configv1alpha1.Condition{ @@ -713,7 +712,7 @@ func appendIdentityProviderObjectRefAPIGroupSuffixCondition(expectedSuffixName s 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", + Message: fmt.Sprintf("some API groups specified by .spec.identityProviders[].objectRef.apiGroup are not recognized (should be %q): %s", expectedSuffixName, strings.Join(sortAndQuote(badSuffixNames), ", ")), }) } else { @@ -727,13 +726,13 @@ func appendIdentityProviderObjectRefAPIGroupSuffixCondition(expectedSuffixName s return conditions } -func appendTransformsExpressionsValidCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { - if len(errors) > 0 { +func appendTransformsExpressionsValidCondition(messages []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { + if len(messages) > 0 { conditions = append(conditions, &configv1alpha1.Condition{ Type: typeTransformsExpressionsValid, Status: configv1alpha1.ConditionFalse, Reason: reasonInvalidTransformsExpressions, - Message: strings.Join(errors, "\n\n"), + Message: strings.Join(messages, "\n\n"), }) } else { conditions = append(conditions, &configv1alpha1.Condition{ @@ -746,13 +745,13 @@ func appendTransformsExpressionsValidCondition(errors []string, conditions []*co return conditions } -func appendTransformsExamplesPassedCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { - if len(errors) > 0 { +func appendTransformsExamplesPassedCondition(messages []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { + if len(messages) > 0 { conditions = append(conditions, &configv1alpha1.Condition{ Type: typeTransformsExamplesPassed, Status: configv1alpha1.ConditionFalse, Reason: reasonTransformsExamplesFailed, - Message: strings.Join(errors, "\n\n"), + Message: strings.Join(messages, "\n\n"), }) } else { conditions = append(conditions, &configv1alpha1.Condition{ @@ -765,13 +764,13 @@ func appendTransformsExamplesPassedCondition(errors []string, conditions []*conf return conditions } -func appendTransformsConstantsNamesUniqueCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { - if len(errors) > 0 { +func appendTransformsConstantsNamesUniqueCondition(messages []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { + if len(messages) > 0 { conditions = append(conditions, &configv1alpha1.Condition{ Type: typeTransformsConstantsNamesUnique, Status: configv1alpha1.ConditionFalse, Reason: reasonDuplicateConstantsNames, - Message: strings.Join(errors, "\n\n"), + Message: strings.Join(messages, "\n\n"), }) } else { conditions = append(conditions, &configv1alpha1.Condition{ diff --git a/internal/controller/supervisorconfig/federation_domain_watcher_test.go b/internal/controller/supervisorconfig/federation_domain_watcher_test.go index 504e3596e..5790ad9e1 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher_test.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher_test.go @@ -367,14 +367,14 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { } } - sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound := func(idpsNotFound string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition { + sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound := func(errorMessages string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition { return configv1alpha1.Condition{ Type: "IdentityProvidersFound", Status: "False", ObservedGeneration: observedGeneration, LastTransitionTime: time, Reason: "IdentityProvidersObjectRefsNotFound", - Message: fmt.Sprintf(".spec.identityProviders[].objectRef identifies resource(s) that cannot be found: %s", idpsNotFound), + Message: errorMessages, } } @@ -484,7 +484,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObservedGeneration: observedGeneration, LastTransitionTime: time, Reason: "APIGroupUnrecognized", - Message: fmt.Sprintf("the API groups specified by .spec.identityProviders[].objectRef.apiGroup "+ + Message: fmt.Sprintf("some API groups specified by .spec.identityProviders[].objectRef.apiGroup "+ "are not recognized (should be \"idp.supervisor.%s\"): %s", apiGroupSuffix, badApiGroups), } } @@ -507,7 +507,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ObservedGeneration: observedGeneration, LastTransitionTime: time, Reason: "KindUnrecognized", - Message: fmt.Sprintf(`the kinds specified by .spec.identityProviders[].objectRef.kind are `+ + Message: fmt.Sprintf(`some kinds specified by .spec.identityProviders[].objectRef.kind are `+ `not recognized (should be one of "ActiveDirectoryIdentityProvider", "LDAPIdentityProvider", "OIDCIdentityProvider"): %s`, badKinds), } } @@ -1051,11 +1051,13 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { replaceConditions( allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []configv1alpha1.Condition{ - sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound( - `.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), + sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(here.Doc( + `cannot find resource specified by .spec.identityProviders[0].objectRef (with name "cant-find-me-name") + + cannot find resource specified by .spec.identityProviders[1].objectRef (with name "cant-find-me-either-name") + + cannot find resource specified by .spec.identityProviders[2].objectRef (with name "cant-find-me-still-name")`, + ), frozenMetav1Now, 123), sadReadyCondition(frozenMetav1Now, 123), }), ), @@ -1267,11 +1269,13 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []configv1alpha1.Condition{ sadAPIGroupSuffixCondition(`"", "", "wrong.example.com"`, frozenMetav1Now, 123), - sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound( - `.spec.identityProviders[0] with displayName "name1", `+ - `.spec.identityProviders[1] with displayName "name2", `+ - `.spec.identityProviders[2] with displayName "name3"`, - frozenMetav1Now, 123), + sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(here.Doc( + `cannot find resource specified by .spec.identityProviders[0].objectRef (with name "some-oidc-idp") + + cannot find resource specified by .spec.identityProviders[1].objectRef (with name "some-ldap-idp") + + cannot find resource specified by .spec.identityProviders[2].objectRef (with name "some-ldap-idp")`, + ), frozenMetav1Now, 123), sadReadyCondition(frozenMetav1Now, 123), }), ), @@ -1327,10 +1331,11 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), []configv1alpha1.Condition{ sadKindCondition(`"", "wrong"`, frozenMetav1Now, 123), - sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound( - `.spec.identityProviders[1] with displayName "name2", `+ - `.spec.identityProviders[2] with displayName "name3"`, - frozenMetav1Now, 123), + sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(here.Doc( + `cannot find resource specified by .spec.identityProviders[1].objectRef (with name "some-ldap-idp") + + cannot find resource specified by .spec.identityProviders[2].objectRef (with name "some-ldap-idp")`, + ), frozenMetav1Now, 123), sadReadyCondition(frozenMetav1Now, 123), }), ), @@ -1823,9 +1828,13 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) { ), frozenMetav1Now, 123), sadAPIGroupSuffixCondition(`"this is wrong"`, frozenMetav1Now, 123), sadDisplayNamesUniqueCondition(`"not unique"`, frozenMetav1Now, 123), - sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound( - `.spec.identityProviders[0] with displayName "not unique", .spec.identityProviders[1] with displayName "not unique", .spec.identityProviders[2] with displayName "name1"`, - frozenMetav1Now, 123), + sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(here.Doc( + `cannot find resource specified by .spec.identityProviders[0].objectRef (with name "this will not be found") + + cannot find resource specified by .spec.identityProviders[1].objectRef (with name "foo") + + cannot find resource specified by .spec.identityProviders[2].objectRef (with name "foo")`, + ), frozenMetav1Now, 123), sadIssuerIsUniqueCondition(frozenMetav1Now, 123), sadKindCondition(`"this is wrong"`, frozenMetav1Now, 123), sadTransformationExpressionsCondition(here.Doc( diff --git a/test/integration/supervisor_federationdomain_status_test.go b/test/integration/supervisor_federationdomain_status_test.go index 762b95048..e630d4823 100644 --- a/test/integration/supervisor_federationdomain_status_test.go +++ b/test/integration/supervisor_federationdomain_status_test.go @@ -6,6 +6,7 @@ package integration import ( "context" "fmt" + "go.pinniped.dev/internal/here" "testing" "time" @@ -120,7 +121,11 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { []v1alpha1.Condition{ { Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProvidersObjectRefsNotFound", - Message: `.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: .spec.identityProviders[0] with displayName "idp1", .spec.identityProviders[1] with displayName "idp2"`, + Message: here.Docf(` + cannot find resource specified by .spec.identityProviders[0].objectRef (with name "%s") + + cannot find resource specified by .spec.identityProviders[1].objectRef (with name "%s")`, + oidcIDP1Meta.Name, oidcIDP2Meta.Name), }, { Type: "Ready", Status: "False", Reason: "NotReady", @@ -140,7 +145,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { []v1alpha1.Condition{ { Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProvidersObjectRefsNotFound", - Message: `.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: .spec.identityProviders[1] with displayName "idp2"`, + Message: fmt.Sprintf(`cannot find resource specified by .spec.identityProviders[1].objectRef (with name "%s")`, oidcIDP2Meta.Name), }, { Type: "Ready", Status: "False", Reason: "NotReady", @@ -168,7 +173,7 @@ func TestSupervisorFederationDomainStatus_Disruptive(t *testing.T) { []v1alpha1.Condition{ { Type: "IdentityProvidersFound", Status: "False", Reason: "IdentityProvidersObjectRefsNotFound", - Message: `.spec.identityProviders[].objectRef identifies resource(s) that cannot be found: .spec.identityProviders[0] with displayName "idp1"`, + Message: fmt.Sprintf(`cannot find resource specified by .spec.identityProviders[0].objectRef (with name "%s")`, oidcIDP1Meta.Name), }, { Type: "Ready", Status: "False", Reason: "NotReady", diff --git a/test/testlib/client.go b/test/testlib/client.go index c80c2e20d..6053a671d 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -354,7 +354,7 @@ func WaitForFederationDomainStatusConditions(ctx context.Context, t *testing.T, "wanted status conditions: %#v\nactual status conditions were: %#v\nnot equal at index %d", expectConditions, fd.Status.Conditions, i) } - }, 60*time.Second, 1*time.Second, "wanted FederationDomain conditions") + }, 5*time.Second, 1*time.Second, "wanted FederationDomain conditions") } func RandBytes(t *testing.T, numBytes int) []byte {