diff --git a/pkg/controller/istiocsr/utils.go b/pkg/controller/istiocsr/utils.go index 896c8bbe6..0d669716b 100644 --- a/pkg/controller/istiocsr/utils.go +++ b/pkg/controller/istiocsr/utils.go @@ -482,7 +482,7 @@ func (r *Reconciler) updateCondition(istiocsr *v1alpha1.IstioCSR, prependErr err if err := r.updateStatus(r.ctx, istiocsr); err != nil { errUpdate := fmt.Errorf("failed to update %s/%s status: %w", istiocsr.GetNamespace(), istiocsr.GetName(), err) if prependErr != nil { - return utilerrors.NewAggregate([]error{err, errUpdate}) + return utilerrors.NewAggregate([]error{prependErr, errUpdate}) } return errUpdate } diff --git a/pkg/controller/istiocsr/utils_test.go b/pkg/controller/istiocsr/utils_test.go index d141e20bc..81cf8999c 100644 --- a/pkg/controller/istiocsr/utils_test.go +++ b/pkg/controller/istiocsr/utils_test.go @@ -1,6 +1,7 @@ package istiocsr import ( + "context" "fmt" "strings" "testing" @@ -10,9 +11,13 @@ import ( networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" ) // baseDeployment returns a minimal deployment for spec comparison tests. @@ -529,3 +534,80 @@ func TestNetworkPolicySpecModified(t *testing.T) { }) } } + +func TestUpdateCondition(t *testing.T) { + tests := []struct { + name string + prependErr error + statusErr bool + wantErr bool + wantMsg string + }{ + { + name: "status update succeeds with nil prependErr", + prependErr: nil, + wantErr: false, + }, + { + name: "status update succeeds with prependErr", + prependErr: fmt.Errorf("original reconcile error"), + wantErr: true, + wantMsg: "original reconcile error", + }, + { + name: "status update fails with nil prependErr", + statusErr: true, + wantErr: true, + wantMsg: "failed to update", + }, + { + name: "status update fails with prependErr preserves both errors", + prependErr: fmt.Errorf("original reconcile error"), + statusErr: true, + wantErr: true, + wantMsg: "original reconcile error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := &fakes.FakeCtrlClient{} + + if tt.statusErr { + fakeClient.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { + return fmt.Errorf("simulated get error") + }) + } else { + fakeClient.GetCalls(func(_ context.Context, _ types.NamespacedName, obj client.Object) error { + switch o := obj.(type) { + case *v1alpha1.IstioCSR: + testIstioCSR().DeepCopyInto(o) + } + return nil + }) + fakeClient.StatusUpdateReturns(nil) + } + + r := testReconciler(t) + r.CtrlClient = fakeClient + + istiocsr := testIstioCSR() + err := r.updateCondition(istiocsr, tt.prependErr) + + if tt.wantErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tt.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.wantMsg != "" && !strings.Contains(err.Error(), tt.wantMsg) { + t.Errorf("error %q should contain %q", err.Error(), tt.wantMsg) + } + if tt.name == "status update fails with prependErr preserves both errors" { + if !strings.Contains(err.Error(), "failed to update") { + t.Errorf("error %q should also contain status update failure", err.Error()) + } + } + }) + } +}