Skip to content

Commit

Permalink
Use custom mocks instead of mocks.StorageAuthority (letsencrypt#7494)
Browse files Browse the repository at this point in the history
Replace "mocks.StorageAuthority" with "sapb.StorageAuthorityClient" in
our test mocks. The improves them by removing implementations of the
methods the tests don't actually need, instead of inheriting lots of
extraneous methods from the huge and cumbersome mocks.StorageAuthority.

This reduces our usage of mocks.StorageAuthority to only the WFE tests
(which create one in the frequently-used setup() function), which will
make refactoring those mocks in the pursuit of
letsencrypt#7476 much easier.

Part of letsencrypt#7476
  • Loading branch information
aarongable authored May 21, 2024
1 parent d2d4f4a commit 4663b98
Show file tree
Hide file tree
Showing 12 changed files with 404 additions and 322 deletions.
14 changes: 13 additions & 1 deletion cmd/admin/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@ func (msa *mockSARecordingBlocks) reset() {
msa.blockRequests = nil
}

type mockSARO struct {
sapb.StorageAuthorityReadOnlyClient
}

func (sa *mockSARO) GetSerialsByKey(ctx context.Context, _ *sapb.SPKIHash, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByKeyClient, error) {
return &mocks.ServerStreamClient[sapb.Serial]{}, nil
}

func (sa *mockSARO) KeyBlocked(ctx context.Context, req *sapb.SPKIHash, _ ...grpc.CallOption) (*sapb.Exists, error) {
return &sapb.Exists{Exists: false}, nil
}

func TestBlockSPKIHash(t *testing.T) {
fc := clock.NewFake()
fc.Set(time.Now())
Expand All @@ -97,7 +109,7 @@ func TestBlockSPKIHash(t *testing.T) {
keyHash, err := core.KeyDigest(privKey.Public())
test.AssertNotError(t, err, "computing test SPKI hash")

a := admin{saroc: &mocks.StorageAuthorityReadOnly{}, sac: &msa, clk: fc, log: log}
a := admin{saroc: &mockSARO{}, sac: &msa, clk: fc, log: log}
u := &user.User{}

// A full run should result in one request with the right fields.
Expand Down
6 changes: 3 additions & 3 deletions crl/updater/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ import (
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/jmhodges/clock"
"github.com/prometheus/client_golang/prometheus"

capb "github.com/letsencrypt/boulder/ca/proto"
corepb "github.com/letsencrypt/boulder/core/proto"
cspb "github.com/letsencrypt/boulder/crl/storer/proto"
"github.com/letsencrypt/boulder/issuance"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/metrics"
"github.com/letsencrypt/boulder/mocks"
sapb "github.com/letsencrypt/boulder/sa/proto"
"github.com/letsencrypt/boulder/test"
"github.com/prometheus/client_golang/prometheus"
)

// fakeGRCC is a fake sapb.StorageAuthority_GetRevokedCertsClient which can be
Expand Down Expand Up @@ -50,7 +50,7 @@ func (f *fakeGRCC) Recv() (*corepb.CRLEntry, error) {
// fakeGRCC to be used as the return value for calls to GetRevokedCerts, and a
// fake timestamp to serve as the database's maximum notAfter value.
type fakeSAC struct {
mocks.StorageAuthority
sapb.StorageAuthorityClient
grcc fakeGRCC
maxNotAfter time.Time
leaseError error
Expand Down
19 changes: 19 additions & 0 deletions mocks/grpc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package mocks

import (
"io"

"google.golang.org/grpc"
)

// ServerStreamClient is a mock which satisfies the grpc.ClientStream interface,
// allowing it to be returned by methods where the server returns a stream of
// results. This simple mock will always return zero results.
type ServerStreamClient[T any] struct {
grpc.ClientStream
}

// Recv immediately returns the EOF error, indicating that the stream is done.
func (c *ServerStreamClient[T]) Recv() (*T, error) {
return nil, io.EOF
}
60 changes: 60 additions & 0 deletions mocks/mailer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package mocks

import (
"sync"

"github.com/letsencrypt/boulder/mail"
)

// Mailer is a mock
type Mailer struct {
sync.Mutex
Messages []MailerMessage
}

var _ mail.Mailer = &Mailer{}

// mockMailerConn is a mock that satisfies the mail.Conn interface
type mockMailerConn struct {
parent *Mailer
}

var _ mail.Conn = &mockMailerConn{}

// MailerMessage holds the captured emails from SendMail()
type MailerMessage struct {
To string
Subject string
Body string
}

// Clear removes any previously recorded messages
func (m *Mailer) Clear() {
m.Lock()
defer m.Unlock()
m.Messages = nil
}

// SendMail is a mock
func (m *mockMailerConn) SendMail(to []string, subject, msg string) error {
m.parent.Lock()
defer m.parent.Unlock()
for _, rcpt := range to {
m.parent.Messages = append(m.parent.Messages, MailerMessage{
To: rcpt,
Subject: subject,
Body: msg,
})
}
return nil
}

// Close is a mock
func (m *mockMailerConn) Close() error {
return nil
}

// Connect is a mock
func (m *Mailer) Connect() (mail.Conn, error) {
return &mockMailerConn{parent: m}, nil
}
19 changes: 19 additions & 0 deletions mocks/publisher.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package mocks

import (
"context"

"google.golang.org/grpc"

pubpb "github.com/letsencrypt/boulder/publisher/proto"
)

// PublisherClient is a mock
type PublisherClient struct {
// empty
}

// SubmitToSingleCTWithResult is a mock
func (*PublisherClient) SubmitToSingleCTWithResult(_ context.Context, _ *pubpb.Request, _ ...grpc.CallOption) (*pubpb.Result, error) {
return &pubpb.Result{}, nil
}
95 changes: 8 additions & 87 deletions mocks/mocks.go → mocks/sa.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ import (
"crypto/x509"
"errors"
"fmt"
"io"
"math/rand"
"net"
"os"
"sync"
"time"

"github.com/go-jose/go-jose/v4"
Expand All @@ -26,8 +24,6 @@ import (
berrors "github.com/letsencrypt/boulder/errors"
bgrpc "github.com/letsencrypt/boulder/grpc"
"github.com/letsencrypt/boulder/identifier"
"github.com/letsencrypt/boulder/mail"
pubpb "github.com/letsencrypt/boulder/publisher/proto"
sapb "github.com/letsencrypt/boulder/sa/proto"
)

Expand All @@ -53,18 +49,6 @@ func NewStorageAuthority(clk clock.Clock) *StorageAuthority {
return &StorageAuthority{StorageAuthorityReadOnly{clk}}
}

// serverStreamClient is a mock which satisfies the grpc.ClientStream interface,
// allowing it to be returned by methods where the server returns a stream of
// results. This simple mock will always return zero results.
type serverStreamClient[T any] struct {
grpc.ClientStream
}

// Recv immediately returns the EOF error, indicating that the stream is done.
func (c *serverStreamClient[T]) Recv() (*T, error) {
return nil, io.EOF
}

const (
test1KeyPublicJSON = `{"kty":"RSA","n":"yNWVhtYEKJR21y9xsHV-PD_bYwbXSeNuFal46xYxVfRL5mqha7vttvjB_vc7Xg2RvgCxHPCqoxgMPTzHrZT75LjCwIW2K_klBYN8oYvTwwmeSkAz6ut7ZxPv-nZaT5TJhGk0NT2kh_zSpdriEJ_3vW-mqxYbbBmpvHqsa1_zx9fSuHYctAZJWzxzUZXykbWMWQZpEiE0J4ajj51fInEzVn7VxV-mzfMyboQjujPh7aNJxAWSq4oQEJJDgWwSh9leyoJoPpONHxh5nEE5AjE01FkGICSxjpZsF-w8hOTI3XXohUdu29Se26k2B0PolDSuj0GIQU6-W9TdLXSjBb2SpQ","e":"AQAB"}`
test2KeyPublicJSON = `{"kty":"RSA","n":"qnARLrT7Xz4gRcKyLdydmCr-ey9OuPImX4X40thk3on26FkMznR3fRjs66eLK7mmPcBZ6uOJseURU6wAaZNmemoYx1dMvqvWWIyiQleHSD7Q8vBrhR6uIoO4jAzJZR-ChzZuSDt7iHN-3xUVspu5XGwXU_MVJZshTwp4TaFx5elHIT_ObnTvTOU3Xhish07AbgZKmWsVbXh5s-CrIicU4OexJPgunWZ_YJJueOKmTvnLlTV4MzKR2oZlBKZ27S0-SfdV_QDx_ydle5oMAyKVtlAV35cyPMIsYNwgUGBCdY_2Uzi5eX0lTc7MPRwz6qR1kip-i59VcGcUQgqHV6Fyqw","e":"AQAB"}`
Expand Down Expand Up @@ -252,22 +236,22 @@ func (sa *StorageAuthorityReadOnly) GetRevocationStatus(_ context.Context, req *

// SerialsForIncident is a mock
func (sa *StorageAuthorityReadOnly) SerialsForIncident(ctx context.Context, _ *sapb.SerialsForIncidentRequest, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_SerialsForIncidentClient, error) {
return &serverStreamClient[sapb.IncidentSerial]{}, nil
return &ServerStreamClient[sapb.IncidentSerial]{}, nil
}

// SerialsForIncident is a mock
func (sa *StorageAuthority) SerialsForIncident(ctx context.Context, _ *sapb.SerialsForIncidentRequest, _ ...grpc.CallOption) (sapb.StorageAuthority_SerialsForIncidentClient, error) {
return &serverStreamClient[sapb.IncidentSerial]{}, nil
return &ServerStreamClient[sapb.IncidentSerial]{}, nil
}

// GetRevokedCerts is a mock
func (sa *StorageAuthorityReadOnly) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRequest, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetRevokedCertsClient, error) {
return &serverStreamClient[corepb.CRLEntry]{}, nil
return &ServerStreamClient[corepb.CRLEntry]{}, nil
}

// GetRevokedCerts is a mock
func (sa *StorageAuthority) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRequest, _ ...grpc.CallOption) (sapb.StorageAuthority_GetRevokedCertsClient, error) {
return &serverStreamClient[corepb.CRLEntry]{}, nil
return &ServerStreamClient[corepb.CRLEntry]{}, nil
}

// GetMaxExpiration is a mock
Expand Down Expand Up @@ -559,22 +543,22 @@ func (sa *StorageAuthorityReadOnly) GetAuthorization2(ctx context.Context, id *s

// GetSerialsByKey is a mock
func (sa *StorageAuthorityReadOnly) GetSerialsByKey(ctx context.Context, _ *sapb.SPKIHash, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByKeyClient, error) {
return &serverStreamClient[sapb.Serial]{}, nil
return &ServerStreamClient[sapb.Serial]{}, nil
}

// GetSerialsByKey is a mock
func (sa *StorageAuthority) GetSerialsByKey(ctx context.Context, _ *sapb.SPKIHash, _ ...grpc.CallOption) (sapb.StorageAuthority_GetSerialsByKeyClient, error) {
return &serverStreamClient[sapb.Serial]{}, nil
return &ServerStreamClient[sapb.Serial]{}, nil
}

// GetSerialsByAccount is a mock
func (sa *StorageAuthorityReadOnly) GetSerialsByAccount(ctx context.Context, _ *sapb.RegistrationID, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByAccountClient, error) {
return &serverStreamClient[sapb.Serial]{}, nil
return &ServerStreamClient[sapb.Serial]{}, nil
}

// GetSerialsByAccount is a mock
func (sa *StorageAuthority) GetSerialsByAccount(ctx context.Context, _ *sapb.RegistrationID, _ ...grpc.CallOption) (sapb.StorageAuthority_GetSerialsByAccountClient, error) {
return &serverStreamClient[sapb.Serial]{}, nil
return &ServerStreamClient[sapb.Serial]{}, nil
}

// RevokeCertificate is a mock
Expand Down Expand Up @@ -616,66 +600,3 @@ func (sa *StorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.Update
func (sa *StorageAuthorityReadOnly) ReplacementOrderExists(ctx context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.Exists, error) {
return nil, nil
}

// PublisherClient is a mock
type PublisherClient struct {
// empty
}

// SubmitToSingleCTWithResult is a mock
func (*PublisherClient) SubmitToSingleCTWithResult(_ context.Context, _ *pubpb.Request, _ ...grpc.CallOption) (*pubpb.Result, error) {
return &pubpb.Result{}, nil
}

// Mailer is a mock
type Mailer struct {
sync.Mutex
Messages []MailerMessage
}

var _ mail.Mailer = &Mailer{}

// mockMailerConn is a mock that satisfies the mail.Conn interface
type mockMailerConn struct {
parent *Mailer
}

var _ mail.Conn = &mockMailerConn{}

// MailerMessage holds the captured emails from SendMail()
type MailerMessage struct {
To string
Subject string
Body string
}

// Clear removes any previously recorded messages
func (m *Mailer) Clear() {
m.Lock()
defer m.Unlock()
m.Messages = nil
}

// SendMail is a mock
func (m *mockMailerConn) SendMail(to []string, subject, msg string) error {
m.parent.Lock()
defer m.parent.Unlock()
for _, rcpt := range to {
m.parent.Messages = append(m.parent.Messages, MailerMessage{
To: rcpt,
Subject: subject,
Body: msg,
})
}
return nil
}

// Close is a mock
func (m *mockMailerConn) Close() error {
return nil
}

// Connect is a mock
func (m *Mailer) Connect() (mail.Conn, error) {
return &mockMailerConn{parent: m}, nil
}
7 changes: 3 additions & 4 deletions ocsp/responder/redis/checked_redis_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
berrors "github.com/letsencrypt/boulder/errors"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/metrics"
"github.com/letsencrypt/boulder/mocks"
"github.com/letsencrypt/boulder/ocsp/responder"
ocsp_test "github.com/letsencrypt/boulder/ocsp/test"
"github.com/letsencrypt/boulder/sa"
Expand Down Expand Up @@ -99,7 +98,7 @@ func (s notFoundSelector) SelectOne(_ context.Context, _ interface{}, _ string,

// echoSA always returns the given revocation status.
type echoSA struct {
mocks.StorageAuthorityReadOnly
sapb.StorageAuthorityReadOnlyClient
status *sapb.RevocationStatus
}

Expand All @@ -109,7 +108,7 @@ func (s *echoSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...g

// errorSA always returns an error.
type errorSA struct {
mocks.StorageAuthorityReadOnly
sapb.StorageAuthorityReadOnlyClient
}

func (s *errorSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.RevocationStatus, error) {
Expand All @@ -118,7 +117,7 @@ func (s *errorSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...

// notFoundSA always returns a NotFound error.
type notFoundSA struct {
mocks.StorageAuthorityReadOnly
sapb.StorageAuthorityReadOnlyClient
}

func (s *notFoundSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.RevocationStatus, error) {
Expand Down
Loading

0 comments on commit 4663b98

Please sign in to comment.