From bb015adf4e374dc3b60bd06e9006f1017d2b6cf5 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Fri, 9 Oct 2020 10:39:17 -0400 Subject: [PATCH] Backfill tests to OIDCProviderConfig controller Signed-off-by: Andrew Keesler --- .../oidcproviderconfig_watcher.go | 5 +- .../oidcproviderconfig_watcher_test.go | 632 +++++++++++++++++- internal/multierror/multierror.go | 20 +- 3 files changed, 637 insertions(+), 20 deletions(-) diff --git a/internal/controller/supervisorconfig/oidcproviderconfig_watcher.go b/internal/controller/supervisorconfig/oidcproviderconfig_watcher.go index 3f246a85..3f21d5fe 100644 --- a/internal/controller/supervisorconfig/oidcproviderconfig_watcher.go +++ b/internal/controller/supervisorconfig/oidcproviderconfig_watcher.go @@ -86,7 +86,7 @@ func (c *oidcProviderConfigWatcherController) Sync(ctx controllerlib.Context) er opc.Namespace, opc.Name, configv1alpha1.DuplicateOIDCProviderStatus, - "Duplicate issuer", + "Duplicate issuer: "+opc.Spec.Issuer, ); err != nil { errs.Add(fmt.Errorf("could not update status: %w", err)) } @@ -107,7 +107,6 @@ func (c *oidcProviderConfigWatcherController) Sync(ctx controllerlib.Context) er continue } - oidcProviders = append(oidcProviders, oidcProvider) if err := c.updateStatus( ctx.Context, opc.Namespace, @@ -116,7 +115,9 @@ func (c *oidcProviderConfigWatcherController) Sync(ctx controllerlib.Context) er "Provider successfully created", ); err != nil { errs.Add(fmt.Errorf("could not update status: %w", err)) + continue } + oidcProviders = append(oidcProviders, oidcProvider) } c.providerSetter.SetProviders(oidcProviders...) diff --git a/internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go b/internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go index 6b76280b..a7f549a8 100644 --- a/internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcproviderconfig_watcher_test.go @@ -5,19 +5,27 @@ package supervisorconfig import ( "context" + "errors" + "reflect" + "sync" "testing" "time" "github.com/sclevine/spec" "github.com/sclevine/spec/report" "github.com/stretchr/testify/require" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/clock" + coretesting "k8s.io/client-go/testing" "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" pinnipedfake "go.pinniped.dev/generated/1.19/client/clientset/versioned/fake" pinnipedinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions" "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/testutil" ) @@ -85,6 +93,8 @@ func (f *fakeProvidersSetter) SetProviders(oidcProviders ...*provider.OIDCProvid func TestSync(t *testing.T) { spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { + const namespace = "some-namespace" + var r *require.Assertions var subject controllerlib.Controller @@ -96,6 +106,7 @@ func TestSync(t *testing.T) { var syncContext *controllerlib.Context var frozenNow time.Time var providersSetter *fakeProvidersSetter + var oidcProviderConfigGVR schema.GroupVersionResource // Defer starting the informers until the last possible moment so that the // nested Before's can keep adding things to the informer caches. @@ -114,7 +125,7 @@ func TestSync(t *testing.T) { Context: timeoutContext, Name: subject.Name(), Key: controllerlib.Key{ - Namespace: "some-namespace", + Namespace: namespace, Name: "config-name", }, } @@ -135,6 +146,12 @@ func TestSync(t *testing.T) { opcInformerClient = pinnipedfake.NewSimpleClientset() opcInformers = pinnipedinformers.NewSharedInformerFactory(opcInformerClient, 0) pinnipedAPIClient = pinnipedfake.NewSimpleClientset() + + oidcProviderConfigGVR = schema.GroupVersionResource{ + Group: v1alpha1.SchemeGroupVersion.Group, + Version: v1alpha1.SchemeGroupVersion.Version, + Resource: "oidcproviderconfigs", + } }) it.After(func() { @@ -142,45 +159,632 @@ func TestSync(t *testing.T) { }) when("there are some valid OIDCProviderConfigs in the informer", func() { + var ( + oidcProviderConfig1 *v1alpha1.OIDCProviderConfig + oidcProviderConfig2 *v1alpha1.OIDCProviderConfig + ) + it.Before(func() { - oidcProviderConfig1 := &v1alpha1.OIDCProviderConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: "some-namespace"}, + oidcProviderConfig1 = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace}, Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://issuer1.com"}, } r.NoError(pinnipedAPIClient.Tracker().Add(oidcProviderConfig1)) r.NoError(opcInformerClient.Tracker().Add(oidcProviderConfig1)) - oidcProviderConfig2 := &v1alpha1.OIDCProviderConfig{ - ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: "some-namespace"}, + oidcProviderConfig2 = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: namespace}, Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://issuer2.com"}, } r.NoError(pinnipedAPIClient.Tracker().Add(oidcProviderConfig2)) r.NoError(opcInformerClient.Tracker().Add(oidcProviderConfig2)) }) - it("calls the ProvidersSetter and updates the OIDCProviderConfigs", func() { + it("calls the ProvidersSetter", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) - r.True(providersSetter.SetProvidersWasCalled) - r.Len(providersSetter.OIDCProvidersReceived, 2) + provider1, err := provider.NewOIDCProvider(oidcProviderConfig1.Spec.Issuer) + r.NoError(err) - // TODO make more assertions about the OIDCProvidersReceived - // TODO make assertions about the expected pinnipedAPIClient.Actions() + provider2, err := provider.NewOIDCProvider(oidcProviderConfig2.Spec.Issuer) + r.NoError(err) + + r.True(providersSetter.SetProvidersWasCalled) + r.ElementsMatch( + []*provider.OIDCProvider{ + provider1, + provider2, + }, + providersSetter.OIDCProvidersReceived, + ) + }) + + it("updates the status to success in the OIDCProviderConfigs", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + oidcProviderConfig1.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig1.Status.Message = "Provider successfully created" + + oidcProviderConfig2.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig2.Status.Message = "Provider successfully created" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig1.Namespace, + oidcProviderConfig1.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig1.Namespace, + oidcProviderConfig1, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig2.Namespace, + oidcProviderConfig2.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig2.Namespace, + oidcProviderConfig2, + ), + } + r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) + }) + + when("one OIDCProviderConfig is already up to date", func() { + it.Before(func() { + oidcProviderConfig1.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig1.Status.Message = "Provider successfully created" + + r.NoError(pinnipedAPIClient.Tracker().Update(oidcProviderConfigGVR, oidcProviderConfig1, oidcProviderConfig1.Namespace)) + r.NoError(opcInformerClient.Tracker().Update(oidcProviderConfigGVR, oidcProviderConfig1, oidcProviderConfig1.Namespace)) + }) + + it("only updates the out-of-date OIDCProviderConfig", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + oidcProviderConfig2.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig2.Status.Message = "Provider successfully created" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig1.Namespace, + oidcProviderConfig1.Name, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig2.Namespace, + oidcProviderConfig2.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig2.Namespace, + oidcProviderConfig2, + ), + } + r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) + }) + + it("calls the ProvidersSetter with both OIDCProviderConfig's", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + provider1, err := provider.NewOIDCProvider(oidcProviderConfig1.Spec.Issuer) + r.NoError(err) + + provider2, err := provider.NewOIDCProvider(oidcProviderConfig2.Spec.Issuer) + r.NoError(err) + + r.True(providersSetter.SetProvidersWasCalled) + r.ElementsMatch( + []*provider.OIDCProvider{ + provider1, + provider2, + }, + providersSetter.OIDCProvidersReceived, + ) + }) + }) + + when("updating only one OIDCProviderConfig fails for a reason other than conflict", func() { + it.Before(func() { + once := sync.Once{} + pinnipedAPIClient.PrependReactor( + "update", + "oidcproviderconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + var err error + once.Do(func() { + err = errors.New("some update error") + }) + return true, nil, err + }, + ) + }) + + it("sets the provider that it could actually update in the API", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "1 error(s):\n- could not update status: some update error") + + provider1, err := provider.NewOIDCProvider(oidcProviderConfig1.Spec.Issuer) + r.NoError(err) + + provider2, err := provider.NewOIDCProvider(oidcProviderConfig2.Spec.Issuer) + r.NoError(err) + + r.True(providersSetter.SetProvidersWasCalled) + r.Len(providersSetter.OIDCProvidersReceived, 1) + r.True( + reflect.DeepEqual(providersSetter.OIDCProvidersReceived[0], provider1) || + reflect.DeepEqual(providersSetter.OIDCProvidersReceived[0], provider2), + ) + }) + + it("returns an error", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "1 error(s):\n- could not update status: some update error") + + oidcProviderConfig1.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig1.Status.Message = "Provider successfully created" + + oidcProviderConfig2.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig2.Status.Message = "Provider successfully created" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig1.Namespace, + oidcProviderConfig1.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig1.Namespace, + oidcProviderConfig1, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig2.Namespace, + oidcProviderConfig2.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig2.Namespace, + oidcProviderConfig2, + ), + } + r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) + }) + }) + }) + + when("there are errors updating the OIDCProviderConfigs", func() { + var ( + oidcProviderConfig *v1alpha1.OIDCProviderConfig + ) + + it.Before(func() { + oidcProviderConfig = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "config", Namespace: namespace}, + Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://issuer.com"}, + } + r.NoError(pinnipedAPIClient.Tracker().Add(oidcProviderConfig)) + r.NoError(opcInformerClient.Tracker().Add(oidcProviderConfig)) }) when("there is a conflict while updating an OIDCProviderConfig", func() { - // TODO write this test + it.Before(func() { + once := sync.Once{} + pinnipedAPIClient.PrependReactor( + "update", + "oidcproviderconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + var err error + once.Do(func() { + err = k8serrors.NewConflict(schema.GroupResource{}, "", nil) + }) + return true, nil, err + }, + ) + }) + + it("retries updating the OIDCProviderConfig", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + oidcProviderConfig.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig.Status.Message = "Provider successfully created" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig, + ), + } + r.Equal(expectedActions, pinnipedAPIClient.Actions()) + }) + }) + + when("updating the OIDCProviderConfig fails for a reason other than conflict", func() { + it.Before(func() { + pinnipedAPIClient.PrependReactor( + "update", + "oidcproviderconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some update error") + }, + ) + }) + + it("returns an error", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "1 error(s):\n- could not update status: some update error") + + oidcProviderConfig.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig.Status.Message = "Provider successfully created" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig, + ), + } + r.Equal(expectedActions, pinnipedAPIClient.Actions()) + }) + }) + + when("there is an error when getting the OIDCProviderConfig", func() { + it.Before(func() { + pinnipedAPIClient.PrependReactor( + "get", + "oidcproviderconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some get error") + }, + ) + }) + + it("returns the get error", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "1 error(s):\n- could not update status: get failed: some get error") + + oidcProviderConfig.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig.Status.Message = "Provider successfully created" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig.Name, + ), + } + r.Equal(expectedActions, pinnipedAPIClient.Actions()) + }) }) }) when("there are both valid and invalid OIDCProviderConfigs in the informer", func() { - // TODO write this test + var ( + validOIDCProviderConfig *v1alpha1.OIDCProviderConfig + invalidOIDCProviderConfig *v1alpha1.OIDCProviderConfig + ) + + it.Before(func() { + validOIDCProviderConfig = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "valid-config", Namespace: namespace}, + Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://valid-issuer.com"}, + } + r.NoError(pinnipedAPIClient.Tracker().Add(validOIDCProviderConfig)) + r.NoError(opcInformerClient.Tracker().Add(validOIDCProviderConfig)) + + invalidOIDCProviderConfig = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "invalid-config", Namespace: namespace}, + Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://invalid-issuer.com?some=query"}, + } + r.NoError(pinnipedAPIClient.Tracker().Add(invalidOIDCProviderConfig)) + r.NoError(opcInformerClient.Tracker().Add(invalidOIDCProviderConfig)) + }) + + it("calls the ProvidersSetter with the valid provider", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + validProvider, err := provider.NewOIDCProvider(validOIDCProviderConfig.Spec.Issuer) + r.NoError(err) + + r.True(providersSetter.SetProvidersWasCalled) + r.Equal( + []*provider.OIDCProvider{ + validProvider, + }, + providersSetter.OIDCProvidersReceived, + ) + }) + + it("updates the status to success/invalid in the OIDCProviderConfigs", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + validOIDCProviderConfig.Status.Status = v1alpha1.SuccessOIDCProviderStatus + validOIDCProviderConfig.Status.Message = "Provider successfully created" + + invalidOIDCProviderConfig.Status.Status = v1alpha1.InvalidOIDCProviderStatus + invalidOIDCProviderConfig.Status.Message = "Invalid: issuer must not have query" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + invalidOIDCProviderConfig.Namespace, + invalidOIDCProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + invalidOIDCProviderConfig.Namespace, + invalidOIDCProviderConfig, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + validOIDCProviderConfig.Namespace, + validOIDCProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + validOIDCProviderConfig.Namespace, + validOIDCProviderConfig, + ), + } + r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) + }) + + when("updating only the invalid OIDCProviderConfig fails for a reason other than conflict", func() { + it.Before(func() { + pinnipedAPIClient.PrependReactor( + "update", + "oidcproviderconfigs", + func(action coretesting.Action) (bool, runtime.Object, error) { + updateAction := action.(coretesting.UpdateActionImpl) + opc := updateAction.Object.(*v1alpha1.OIDCProviderConfig) + if opc.Name == validOIDCProviderConfig.Name { + return true, nil, nil + } + + return true, nil, errors.New("some update error") + }, + ) + }) + + it("sets the provider that it could actually update in the API", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "1 error(s):\n- could not update status: some update error") + + validProvider, err := provider.NewOIDCProvider(validOIDCProviderConfig.Spec.Issuer) + r.NoError(err) + + r.True(providersSetter.SetProvidersWasCalled) + r.Equal( + []*provider.OIDCProvider{ + validProvider, + }, + providersSetter.OIDCProvidersReceived, + ) + }) + + it("returns an error", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "1 error(s):\n- could not update status: some update error") + + validOIDCProviderConfig.Status.Status = v1alpha1.SuccessOIDCProviderStatus + validOIDCProviderConfig.Status.Message = "Provider successfully created" + + invalidOIDCProviderConfig.Status.Status = v1alpha1.InvalidOIDCProviderStatus + invalidOIDCProviderConfig.Status.Message = "Invalid: issuer must not have query" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + invalidOIDCProviderConfig.Namespace, + invalidOIDCProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + invalidOIDCProviderConfig.Namespace, + invalidOIDCProviderConfig, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + validOIDCProviderConfig.Namespace, + validOIDCProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + validOIDCProviderConfig.Namespace, + validOIDCProviderConfig, + ), + } + r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) + }) + }) }) - when("they there are OIDCProviderConfigs with duplicate issuer names in the informer", func() { - // TODO write this test + when("there are OIDCProviderConfigs with duplicate issuer names in the informer", func() { + var ( + oidcProviderConfigDuplicate1 *v1alpha1.OIDCProviderConfig + oidcProviderConfigDuplicate2 *v1alpha1.OIDCProviderConfig + oidcProviderConfig *v1alpha1.OIDCProviderConfig + ) + + it.Before(func() { + oidcProviderConfigDuplicate1 = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "duplicate1", Namespace: namespace}, + Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://issuer-duplicate.com"}, + } + r.NoError(pinnipedAPIClient.Tracker().Add(oidcProviderConfigDuplicate1)) + r.NoError(opcInformerClient.Tracker().Add(oidcProviderConfigDuplicate1)) + oidcProviderConfigDuplicate2 = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "duplicate2", Namespace: namespace}, + Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://issuer-duplicate.com"}, + } + r.NoError(pinnipedAPIClient.Tracker().Add(oidcProviderConfigDuplicate2)) + r.NoError(opcInformerClient.Tracker().Add(oidcProviderConfigDuplicate2)) + + oidcProviderConfig = &v1alpha1.OIDCProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "not-duplicate", Namespace: namespace}, + Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://issuer-not-duplicate.com"}, + } + r.NoError(pinnipedAPIClient.Tracker().Add(oidcProviderConfig)) + r.NoError(opcInformerClient.Tracker().Add(oidcProviderConfig)) + }) + + it("calls the ProvidersSetter with the non-duplicate", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + nonDuplicateProvider, err := provider.NewOIDCProvider(oidcProviderConfig.Spec.Issuer) + r.NoError(err) + + r.True(providersSetter.SetProvidersWasCalled) + r.Equal( + []*provider.OIDCProvider{ + nonDuplicateProvider, + }, + providersSetter.OIDCProvidersReceived, + ) + }) + + it("updates the statuses", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + oidcProviderConfig.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig.Status.Message = "Provider successfully created" + + oidcProviderConfigDuplicate1.Status.Status = v1alpha1.DuplicateOIDCProviderStatus + oidcProviderConfigDuplicate1.Status.Message = "Duplicate issuer: https://issuer-duplicate.com" + + oidcProviderConfigDuplicate2.Status.Status = v1alpha1.DuplicateOIDCProviderStatus + oidcProviderConfigDuplicate2.Status.Message = "Duplicate issuer: https://issuer-duplicate.com" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfigDuplicate1.Namespace, + oidcProviderConfigDuplicate1.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfigDuplicate1.Namespace, + oidcProviderConfigDuplicate1, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfigDuplicate2.Namespace, + oidcProviderConfigDuplicate2.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfigDuplicate2.Namespace, + oidcProviderConfigDuplicate2, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig.Name, + ), + coretesting.NewUpdateAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig, + ), + } + r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) + }) + + when("we cannot talk to the API", func() { + it.Before(func() { + pinnipedAPIClient.PrependReactor( + "get", + "oidcproviderconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some get error") + }, + ) + }) + + it("returns the get errors", func() { + expectedError := here.Doc(` + 3 error(s): + - could not update status: get failed: some get error + - could not update status: get failed: some get error + - could not update status: get failed: some get error`) + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, expectedError) + + oidcProviderConfig.Status.Status = v1alpha1.SuccessOIDCProviderStatus + oidcProviderConfig.Status.Message = "Provider successfully created" + + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfigDuplicate1.Namespace, + oidcProviderConfigDuplicate1.Name, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfigDuplicate2.Namespace, + oidcProviderConfigDuplicate2.Name, + ), + coretesting.NewGetAction( + oidcProviderConfigGVR, + oidcProviderConfig.Namespace, + oidcProviderConfig.Name, + ), + } + r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions()) + }) + }) }) when("there are no OIDCProviderConfigs in the informer", func() { diff --git a/internal/multierror/multierror.go b/internal/multierror/multierror.go index c9eab6b9..5f87107f 100644 --- a/internal/multierror/multierror.go +++ b/internal/multierror/multierror.go @@ -17,6 +17,14 @@ import ( "strings" ) +// formatFunc is a function used to format the string representing of a MultiError. It is used in the +// Error() function. +// +// It is marked out here to indicate how we could potentially extend MultiError in the future to +// support more styles of converting from a list of error's to a string. +//nolint: gochecknoglobals +var formatFunc func(errs MultiError, sb *strings.Builder) = defaultFormat + // MultiError holds a list of error's, that could potentially be empty. // // Use New() to create a MultiError. @@ -35,10 +43,7 @@ func (m *MultiError) Add(err error) { // Error implements the error.Error() interface method. func (m MultiError) Error() string { sb := strings.Builder{} - _, _ = fmt.Fprintf(&sb, "%d error(s):", len(m)) - for _, err := range m { - _, _ = fmt.Fprintf(&sb, "\n- %s", err.Error()) - } + formatFunc(m, &sb) return sb.String() } @@ -49,3 +54,10 @@ func (m MultiError) ErrOrNil() error { } return nil } + +func defaultFormat(errs MultiError, sb *strings.Builder) { + _, _ = fmt.Fprintf(sb, "%d error(s):", len(errs)) + for _, err := range errs { + _, _ = fmt.Fprintf(sb, "\n- %s", err.Error()) + } +}