From a181d8b0b632458ca1dc8d6e1cc91a9c69f606d5 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 4 Apr 2016 15:44:11 -0700 Subject: [PATCH] Fix handling of ErrDuplicate in AddSCTReceipt. --- rpc/rpc-wrappers.go | 9 +-------- sa/storage-authority.go | 13 +++++-------- sa/storage-authority_test.go | 4 +--- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/rpc/rpc-wrappers.go b/rpc/rpc-wrappers.go index 400f2ff4d..8812bcd0d 100644 --- a/rpc/rpc-wrappers.go +++ b/rpc/rpc-wrappers.go @@ -1200,14 +1200,7 @@ func NewStorageAuthorityServer(rpc Server, impl core.StorageAuthority) error { return } - err = impl.AddSCTReceipt(core.SignedCertificateTimestamp(sct)) - if err != nil { - // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 - errorCondition(MethodAddSCTReceipt, err, req) - return - } - - return nil, nil + return nil, impl.AddSCTReceipt(core.SignedCertificateTimestamp(sct)) }) rpc.Handle(MethodCountFQDNSets, func(req []byte) (response []byte, err error) { diff --git a/sa/storage-authority.go b/sa/storage-authority.go index ec5b78bb5..b49b5f65e 100644 --- a/sa/storage-authority.go +++ b/sa/storage-authority.go @@ -897,18 +897,15 @@ func (ssa *SQLStorageAuthority) GetSCTReceipt(serial string, logID string) (rece return } -// ErrDuplicateReceipt is an error type for duplicate SCT receipts -type ErrDuplicateReceipt string - -func (e ErrDuplicateReceipt) Error() string { - return string(e) -} - // AddSCTReceipt adds a new SCT receipt to the (append-only) sctReceipts table func (ssa *SQLStorageAuthority) AddSCTReceipt(sct core.SignedCertificateTimestamp) error { err := ssa.dbMap.Insert(&sct) + // For AddSCTReceipt, duplicates are explicitly OK, so don't return errors + // based on duplicates, especially because we currently retry all submissions + // for a certificate if even one of them fails. Once https://github.com/letsencrypt/boulder/issues/891 + // is fixed, we may want to start returning this as an error, or logging it. if err != nil && strings.HasPrefix(err.Error(), "Error 1062: Duplicate entry") { - err = ErrDuplicateReceipt(err.Error()) + return nil } return err } diff --git a/sa/storage-authority_test.go b/sa/storage-authority_test.go index ca57bdd63..0893f3f67 100644 --- a/sa/storage-authority_test.go +++ b/sa/storage-authority_test.go @@ -42,7 +42,6 @@ func initSA(t *testing.T) (*SQLStorageAuthority, clock.FakeClock, func()) { if err != nil { t.Fatalf("Failed to create dbMap: %s", err) } - dbMap.TraceOn("SQL: ", &SQLLogger{log}) fc := clock.NewFake() fc.Set(time.Date(2015, 3, 4, 5, 0, 0, 0, time.UTC)) @@ -471,8 +470,7 @@ func TestAddSCTReceipt(t *testing.T) { test.AssertNotError(t, err, "Failed to add SCT receipt") // Append only and unique on signature and across LogID and CertificateSerial err = sa.AddSCTReceipt(sct) - test.AssertError(t, err, "Incorrectly added duplicate SCT receipt") - fmt.Println(err) + test.AssertNotError(t, err, "Incorrectly returned error on duplicate SCT receipt") } func TestGetSCTReceipt(t *testing.T) {