fix(cfapi): requeue after DB error

internal/cfapi/cfapi_test.go

@@ -9,7 +9,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/google/go-cmp/cmp"
+	"gotest.tools/v3/assert"
 )
 
 func TestSignResponse_Unmarshal(t *testing.T) {
@@ -59,24 +59,20 @@ func TestSignResponse_Unmarshal(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			var resp SignResponse
 
-			if err := json.Unmarshal(tt.payload, &resp); err != nil {
-				t.Fatalf("unable to unmarsahl: %s", err)
-			}
-
-			if diff := cmp.Diff(resp, expected); diff != "" {
-				t.Fatalf("diff: (-want +got)\n%s", diff)
-			}
+			assert.NilError(t, json.Unmarshal(tt.payload, &resp))
+			assert.DeepEqual(t, resp, expected)
 		})
 	}
 }
 
 func TestSign(t *testing.T) {
 	expectedTime := time.Date(2020, time.December, 25, 6, 27, 0, 0, time.UTC)
 	tests := []struct {
-		name     string
-		handler  http.Handler
-		response *SignResponse
-		error    string
+		name      string
+		handler   http.Handler
+		response  *SignResponse
+		error     string
+		errorType error
 	}{
 		{name: "API success",
 			handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -118,8 +114,9 @@ func TestSign(t *testing.T) {
 	"result": {}
 }`)
 			}),
-			response: nil,
-			error:    "Cloudflare API Error code=9001 message=Over Nine Thousand! ray_id=0123456789abcdef-ABC",
+			response:  nil,
+			error:     "Cloudflare API Error code=9001 message=Over Nine Thousand! ray_id=0123456789abcdef-ABC",
+			errorType: &APIError{},
 		},
 	}
 
@@ -140,14 +137,13 @@ func TestSign(t *testing.T) {
 				CSR:       "Lorem ipsum dolor sit amet, consectetur adipiscing elit.",
 			})
 
-			if diff := cmp.Diff(resp, tt.response); diff != "" {
-				t.Fatalf("diff: (-want +got)\n%s", diff)
-			}
+			assert.DeepEqual(t, resp, tt.response)
 
 			if tt.error != "" {
-				if diff := cmp.Diff(err.Error(), tt.error); diff != "" {
-					t.Fatalf("diff: (-want +got)\n%s", diff)
-				}
+				assert.Error(t, err, tt.error)
+				assert.ErrorType(t, err, tt.errorType)
+			} else {
+				assert.NilError(t, err)
 			}
 		})
 	}

pkgs/controllers/certificaterequest.go

@@ -2,11 +2,13 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	cmutil "github.com/cert-manager/cert-manager/pkg/api/util"
 	certmanager "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
+	"github.com/cloudflare/origin-ca-issuer/internal/cfapi"
 	v1 "github.com/cloudflare/origin-ca-issuer/pkgs/apis/v1"
 	"github.com/cloudflare/origin-ca-issuer/pkgs/provisioners"
 	"github.com/go-logr/logr"
@@ -17,6 +19,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
+const originDBWriteErrorCode = 1100
+
 // CertificateRequestController implements a controller that reconciles CertificateRequests
 // that references this controller.
 type CertificateRequestController struct {
@@ -135,6 +139,15 @@ func (r *CertificateRequestController) Reconcile(ctx context.Context, cr *certma
 	}
 
 	pem, err := p.Sign(ctx, cr)
+
+	var apiError *cfapi.APIError
+	if errors.As(err, &apiError) {
+		if apiError.Code == originDBWriteErrorCode {
+			log.Error(err, "requeue-ing after API error")
+			return reconcile.Result{}, err
+		}
+	}
+
 	if err != nil {
 		log.Error(err, "failed to sign certificate request")
 		_ = r.setStatus(ctx, cr, cmmeta.ConditionFalse, certmanager.CertificateRequestReasonFailed, fmt.Sprintf("Failed to sign certificate request: %v", err))

pkgs/controllers/certificaterequest_test.go

@@ -11,10 +11,9 @@ import (
 	cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	cmgen "github.com/cert-manager/cert-manager/test/unit/gen"
 	"github.com/cloudflare/origin-ca-issuer/internal/cfapi"
-	fakeapi "github.com/cloudflare/origin-ca-issuer/internal/cfapi/testing"
 	v1 "github.com/cloudflare/origin-ca-issuer/pkgs/apis/v1"
 	"github.com/cloudflare/origin-ca-issuer/pkgs/provisioners"
-	"github.com/google/go-cmp/cmp"
+	"gotest.tools/v3/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -97,17 +96,17 @@ func TestCertificateRequestReconcile(t *testing.T) {
 						Namespace: "default",
 					},
 					Provisioner: (func() *provisioners.Provisioner {
-						c := &fakeapi.FakeClient{
-							Response: &cfapi.SignResponse{
+						c := SignerFunc(func(ctx context.Context, sr *cfapi.SignRequest) (*cfapi.SignResponse, error) {
+							return &cfapi.SignResponse{
 								Id:          "1",
 								Certificate: "bogus",
 								Hostnames:   []string{"example.com"},
 								Expiration:  time.Time{},
 								Type:        "colemak",
 								Validity:    0,
 								CSR:         "foobar",
-							},
-						}
+							}, nil
+						})
 						p, err := provisioners.New(c, v1.RequestTypeOriginRSA, logf.Log)
 						if err != nil {
 							t.Fatalf("error creating provisioner: %s", err)
@@ -134,6 +133,78 @@ func TestCertificateRequestReconcile(t *testing.T) {
 				Name:      "foobar",
 			},
 		},
+		{
+			name: "requeue after API error",
+			objects: []runtime.Object{
+				cmgen.CertificateRequest("foobar",
+					cmgen.SetCertificateRequestNamespace("default"),
+					cmgen.SetCertificateRequestDuration(&metav1.Duration{Duration: 7 * 24 * time.Hour}),
+					cmgen.SetCertificateRequestCSR((func() []byte {
+						csr, _, err := cmgen.CSR(x509.ECDSA)
+						if err != nil {
+							t.Fatalf("creating CSR: %s", err)
+						}
+
+						return csr
+					})()),
+					cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
+						Name:  "foobar",
+						Kind:  "OriginIssuer",
+						Group: "cert-manager.k8s.cloudflare.com",
+					}),
+				),
+				&v1.OriginIssuer{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "foobar",
+						Namespace: "default",
+					},
+					Spec: v1.OriginIssuerSpec{
+						Auth: v1.OriginIssuerAuthentication{
+							ServiceKeyRef: v1.SecretKeySelector{
+								Name: "service-key-issuer",
+								Key:  "key",
+							},
+						},
+					},
+					Status: v1.OriginIssuerStatus{
+						Conditions: []v1.OriginIssuerCondition{
+							{
+								Type:   v1.ConditionReady,
+								Status: v1.ConditionTrue,
+							},
+						},
+					},
+				},
+			},
+			collection: provisioners.CollectionWith([]provisioners.CollectionItem{
+				{
+					NamespacedName: types.NamespacedName{
+						Name:      "foobar",
+						Namespace: "default",
+					},
+					Provisioner: (func() *provisioners.Provisioner {
+						c := SignerFunc(func(ctx context.Context, sr *cfapi.SignRequest) (*cfapi.SignResponse, error) {
+							return nil, &cfapi.APIError{
+								Code:    1100,
+								Message: "Failed to write certificate to Database",
+								RayID:   "7d3eb086eedab98e",
+							}
+						})
+						p, err := provisioners.New(c, v1.RequestTypeOriginRSA, logf.Log)
+						if err != nil {
+							t.Fatalf("error creating provisioner: %s", err)
+						}
+
+						return p
+					}()),
+				},
+			}),
+			namespaceName: types.NamespacedName{
+				Namespace: "default",
+				Name:      "foobar",
+			},
+			error: "unable to sign request: Cloudflare API Error code=1100 message=Failed to write certificate to Database ray_id=7d3eb086eedab98e",
+		},
 	}
 
 	for _, tt := range tests {
@@ -156,24 +227,20 @@ func TestCertificateRequestReconcile(t *testing.T) {
 			})
 
 			if err != nil {
-				if diff := cmp.Diff(err.Error(), tt.error); diff != "" {
-					t.Fatalf("diff: (-wanted +got)\n%s", diff)
-				}
+				assert.Error(t, err, tt.error)
+			} else {
+				assert.NilError(t, err)
 			}
 
 			got := &cmapi.CertificateRequest{}
-			if err := client.Get(context.TODO(), tt.namespaceName, got); err != nil {
-				t.Fatalf("expected to retrieve issuer from client: %s", err)
-			}
-			if diff := cmp.Diff(got.Status, tt.expected); diff != "" {
-				t.Fatalf("diff: (-want +got)\n%s", diff)
-			}
-
-			if tt.error == "" {
-				if _, ok := controller.Collection.Load(tt.namespaceName); !ok {
-					t.Fatal("was unable to find provisioner")
-				}
-			}
+			assert.NilError(t, client.Get(context.TODO(), tt.namespaceName, got))
+			assert.DeepEqual(t, got.Status, tt.expected)
 		})
 	}
 }
+
+type SignerFunc func(context.Context, *cfapi.SignRequest) (*cfapi.SignResponse, error)
+
+func (f SignerFunc) Sign(ctx context.Context, req *cfapi.SignRequest) (*cfapi.SignResponse, error) {
+	return f(ctx, req)
+}