X Tutup
Skip to content

Commit bab688b

Browse files
aarongableaarongable
authored
Remove sa-wrappers.go (letsencrypt#5663)
Remove the last of the gRPC wrapper files. In order to do so: - Remove the `core.StorageGetter` interface. Replace it with a new interface (whose methods include the `...grpc.CallOption` arg) inside the `sa/proto/` package. - Remove the `core.StorageAdder` interface. There's no real use-case for having a write-only interface. - Remove the `core.StorageAuthority` interface, as it is now redundant with the autogenerated `sapb.StorageAuthorityClient` interface. - Replace the `certificateStorage` interface (which appears in two different places) with a single unified interface also in `sa/proto/`. - Update all test mocks to include the `_ ...grpc.CallOption` arg in their method signatures so they match the gRPC client interface. - Delete many methods from mocks which are no longer necessary (mostly because they're mocking old authz1 methods that no longer exist). - Move the two `test/inmem/` wrappers into their own sub-packages to avoid an import cycle. - Simplify the `satest` package to satisfy one of its TODOs and to avoid an import cycle. - Add many methods to the `test/inmem/sa/` wrapper, to accommodate all of the methods which are called in unittests. Fixes letsencrypt#5600
1 parent b588db1 commit bab688b

File tree

38 files changed

+450
-953
lines changed

38 files changed

+450
-953
lines changed

‎ca/ca.go‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"github.com/miekg/pkcs11"
1919
"github.com/prometheus/client_golang/prometheus"
2020
"golang.org/x/crypto/ocsp"
21-
"google.golang.org/protobuf/types/known/emptypb"
2221

2322
capb "github.com/letsencrypt/boulder/ca/proto"
2423
"github.com/letsencrypt/boulder/core"
@@ -41,13 +40,6 @@ const (
4140
csrExtensionOther = "other"
4241
)
4342

44-
type certificateStorage interface {
45-
AddCertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*sapb.AddCertificateResponse, error)
46-
GetCertificate(ctx context.Context, req *sapb.Serial) (*corepb.Certificate, error)
47-
AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*emptypb.Empty, error)
48-
AddSerial(ctx context.Context, req *sapb.AddSerialRequest) (*emptypb.Empty, error)
49-
}
50-
5143
type certificateType string
5244

5345
const (
@@ -69,7 +61,7 @@ type issuerMaps struct {
6961
type certificateAuthorityImpl struct {
7062
capb.UnimplementedCertificateAuthorityServer
7163
capb.UnimplementedOCSPGeneratorServer
72-
sa certificateStorage
64+
sa sapb.StorageAuthorityCertificateClient
7365
pa core.PolicyAuthority
7466
ocsp *ocspImpl
7567
issuers issuerMaps
@@ -116,7 +108,7 @@ func makeIssuerMaps(issuers []*issuance.Issuer) (issuerMaps, error) {
116108
// from any number of issuance.Issuers according to their profiles, and can sign
117109
// OCSP (via delegation to an ocspImpl and its issuers).
118110
func NewCertificateAuthorityImpl(
119-
sa certificateStorage,
111+
sa sapb.StorageAuthorityCertificateClient,
120112
pa core.PolicyAuthority,
121113
ocsp *ocspImpl,
122114
boulderIssuers []*issuance.Issuer,

‎ca/ca_test.go‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
ctx509 "github.com/google/certificate-transparency-go/x509"
2525
"github.com/jmhodges/clock"
2626
"github.com/prometheus/client_golang/prometheus"
27+
"google.golang.org/grpc"
2728
"google.golang.org/protobuf/types/known/emptypb"
2829

2930
capb "github.com/letsencrypt/boulder/ca/proto"
@@ -150,23 +151,27 @@ type mockSA struct {
150151
certificate core.Certificate
151152
}
152153

153-
func (m *mockSA) AddCertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*sapb.AddCertificateResponse, error) {
154+
func (m *mockSA) AddCertificate(ctx context.Context, req *sapb.AddCertificateRequest, _ ...grpc.CallOption) (*sapb.AddCertificateResponse, error) {
154155
m.certificate.DER = req.Der
155156
return nil, nil
156157
}
157158

158-
func (m *mockSA) AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*emptypb.Empty, error) {
159+
func (m *mockSA) AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest, _ ...grpc.CallOption) (*emptypb.Empty, error) {
159160
return &emptypb.Empty{}, nil
160161
}
161162

162-
func (m *mockSA) AddSerial(ctx context.Context, req *sapb.AddSerialRequest) (*emptypb.Empty, error) {
163+
func (m *mockSA) AddSerial(ctx context.Context, req *sapb.AddSerialRequest, _ ...grpc.CallOption) (*emptypb.Empty, error) {
163164
return &emptypb.Empty{}, nil
164165
}
165166

166-
func (m *mockSA) GetCertificate(ctx context.Context, req *sapb.Serial) (*corepb.Certificate, error) {
167+
func (m *mockSA) GetCertificate(ctx context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*corepb.Certificate, error) {
167168
return nil, berrors.NotFoundError("cannot find the cert")
168169
}
169170

171+
func (m *mockSA) GetPrecertificate(ctx context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*corepb.Certificate, error) {
172+
return nil, berrors.NotFoundError("cannot find the precert")
173+
}
174+
170175
var caKey crypto.Signer
171176
var caCert *issuance.Certificate
172177
var caCert2 *issuance.Certificate
@@ -257,7 +262,6 @@ func setup(t *testing.T) *testCtx {
257262
}, []string{"type"})
258263

259264
ocsp, err := NewOCSPImpl(
260-
&mockSA{},
261265
boulderIssuers,
262266
time.Hour,
263267
0,
@@ -810,7 +814,7 @@ type dupeSA struct {
810814
mockSA
811815
}
812816

813-
func (m *dupeSA) GetCertificate(ctx context.Context, req *sapb.Serial) (*corepb.Certificate, error) {
817+
func (m *dupeSA) GetCertificate(ctx context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*corepb.Certificate, error) {
814818
return nil, nil
815819
}
816820

@@ -819,7 +823,7 @@ type getCertErrorSA struct {
819823
mockSA
820824
}
821825

822-
func (m *getCertErrorSA) GetCertificate(ctx context.Context, req *sapb.Serial) (*corepb.Certificate, error) {
826+
func (m *getCertErrorSA) GetCertificate(ctx context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*corepb.Certificate, error) {
823827
return nil, fmt.Errorf("i don't like it")
824828
}
825829

@@ -912,7 +916,7 @@ type queueSA struct {
912916
issuedPrecert time.Time
913917
}
914918

915-
func (qsa *queueSA) AddCertificate(_ context.Context, req *sapb.AddCertificateRequest) (*sapb.AddCertificateResponse, error) {
919+
func (qsa *queueSA) AddCertificate(_ context.Context, req *sapb.AddCertificateRequest, _ ...grpc.CallOption) (*sapb.AddCertificateResponse, error) {
916920
if qsa.fail {
917921
return nil, errors.New("bad")
918922
} else if qsa.duplicate {
@@ -922,7 +926,7 @@ func (qsa *queueSA) AddCertificate(_ context.Context, req *sapb.AddCertificateRe
922926
return nil, nil
923927
}
924928

925-
func (qsa *queueSA) AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest) (*emptypb.Empty, error) {
929+
func (qsa *queueSA) AddPrecertificate(ctx context.Context, req *sapb.AddCertificateRequest, _ ...grpc.CallOption) (*emptypb.Empty, error) {
926930
if qsa.fail {
927931
return nil, errors.New("bad")
928932
} else if qsa.duplicate {

‎ca/ocsp.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ type ocspIssuerMaps struct {
2929
// ocspImpl provides a backing implementation for the OCSP gRPC service.
3030
type ocspImpl struct {
3131
capb.UnimplementedOCSPGeneratorServer
32-
sa certificateStorage
3332
issuers ocspIssuerMaps
3433
ocspLifetime time.Duration
3534
ocspLogQueue *ocspLogQueue
@@ -54,7 +53,6 @@ func makeOCSPIssuerMaps(issuers []*issuance.Issuer) (ocspIssuerMaps, error) {
5453
}
5554

5655
func NewOCSPImpl(
57-
sa certificateStorage,
5856
issuers []*issuance.Issuer,
5957
ocspLifetime time.Duration,
6058
ocspLogMaxLength int,
@@ -81,7 +79,6 @@ func NewOCSPImpl(
8179
}
8280

8381
oi := &ocspImpl{
84-
sa: sa,
8582
issuers: issuerMaps,
8683
ocspLifetime: ocspLifetime,
8784
ocspLogQueue: ocspLogQueue,

‎cmd/admin-revoker/main.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type config struct {
6060
Syslog cmd.SyslogConfig
6161
}
6262

63-
func setupContext(c config) (rapb.RegistrationAuthorityClient, blog.Logger, *db.WrappedMap, core.StorageAuthority) {
63+
func setupContext(c config) (rapb.RegistrationAuthorityClient, blog.Logger, *db.WrappedMap, sapb.StorageAuthorityClient) {
6464
logger := cmd.NewLogger(c.Syslog)
6565

6666
tlsConfig, err := c.Revoker.TLS.Load()
@@ -86,7 +86,7 @@ func setupContext(c config) (rapb.RegistrationAuthorityClient, blog.Logger, *db.
8686

8787
saConn, err := bgrpc.ClientSetup(c.Revoker.SAService, tlsConfig, clientMetrics, clk)
8888
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
89-
sac := bgrpc.NewStorageAuthorityClient(sapb.NewStorageAuthorityClient(saConn))
89+
sac := sapb.NewStorageAuthorityClient(saConn)
9090

9191
return rac, logger, dbMap, sac
9292
}

‎cmd/admin-revoker/main_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import (
2525
sapb "github.com/letsencrypt/boulder/sa/proto"
2626
"github.com/letsencrypt/boulder/sa/satest"
2727
"github.com/letsencrypt/boulder/test"
28-
"github.com/letsencrypt/boulder/test/inmem"
28+
ira "github.com/letsencrypt/boulder/test/inmem/ra"
29+
isa "github.com/letsencrypt/boulder/test/inmem/sa"
2930
"github.com/letsencrypt/boulder/test/vars"
3031
"google.golang.org/grpc"
3132
"google.golang.org/protobuf/types/known/emptypb"
@@ -59,7 +60,7 @@ func TestRevokeBatch(t *testing.T) {
5960
t.Fatalf("Failed to create SA: %s", err)
6061
}
6162
defer test.ResetSATestDatabase(t)
62-
reg := satest.CreateWorkingRegistration(t, ssa)
63+
reg := satest.CreateWorkingRegistration(t, isa.SA{Impl: ssa})
6364

6465
issuer, err := issuance.LoadCertificate("../../test/hierarchy/int-r3.cert.pem")
6566
test.AssertNotError(t, err, "Failed to load test issuer")
@@ -82,9 +83,9 @@ func TestRevokeBatch(t *testing.T) {
8283
&mockPurger{},
8384
[]*issuance.Certificate{issuer},
8485
)
85-
ra.SA = ssa
86+
ra.SA = isa.SA{Impl: ssa}
8687
ra.CA = &mockCA{}
87-
rac := inmem.RA{Impl: ra}
88+
rac := ira.RA{Impl: ra}
8889

8990
serialFile, err := ioutil.TempFile("", "serials")
9091
test.AssertNotError(t, err, "failed to open temp file")

‎cmd/boulder-ca/main.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,11 @@ func main() {
203203
cmd.FailOnError(err, "TLS config")
204204

205205
clk := cmd.Clock()
206-
207206
clientMetrics := bgrpc.NewClientMetrics(scope)
207+
208208
conn, err := bgrpc.ClientSetup(c.CA.SAService, tlsConfig, clientMetrics, clk)
209209
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
210-
sa := bgrpc.NewStorageAuthorityClient(sapb.NewStorageAuthorityClient(conn))
210+
sa := sapb.NewStorageAuthorityClient(conn)
211211

212212
kp, err := goodkey.NewKeyPolicy(c.CA.WeakKeyFile, c.CA.BlockedKeyFile, sa.KeyBlocked)
213213
cmd.FailOnError(err, "Unable to create key policy")
@@ -240,7 +240,6 @@ func main() {
240240
var wg sync.WaitGroup
241241

242242
ocspi, err := ca.NewOCSPImpl(
243-
sa,
244243
boulderIssuers,
245244
c.CA.LifespanOCSP.Duration,
246245
c.CA.OCSPLogMaxLength,

‎cmd/boulder-ra/main.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,8 @@ func main() {
152152
cmd.FailOnError(err, "TLS config")
153153

154154
clk := cmd.Clock()
155-
156155
clientMetrics := bgrpc.NewClientMetrics(scope)
156+
157157
vaConn, err := bgrpc.ClientSetup(c.RA.VAService, tlsConfig, clientMetrics, clk)
158158
cmd.FailOnError(err, "Unable to create VA client")
159159
vac := vapb.NewVAClient(vaConn)
@@ -163,6 +163,10 @@ func main() {
163163
cmd.FailOnError(err, "Unable to create CA client")
164164
cac := capb.NewCertificateAuthorityClient(caConn)
165165

166+
saConn, err := bgrpc.ClientSetup(c.RA.SAService, tlsConfig, clientMetrics, clk)
167+
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
168+
sac := sapb.NewStorageAuthorityClient(saConn)
169+
166170
var ctp *ctpolicy.CTPolicy
167171
conn, err := bgrpc.ClientSetup(c.RA.PublisherService, tlsConfig, clientMetrics, clk)
168172
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to Publisher")
@@ -205,10 +209,6 @@ func main() {
205209
}
206210
ctp = ctpolicy.New(pubc, c.RA.CTLogGroups2, c.RA.InformationalCTLogs, logger, scope)
207211

208-
saConn, err := bgrpc.ClientSetup(c.RA.SAService, tlsConfig, clientMetrics, clk)
209-
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
210-
sac := bgrpc.NewStorageAuthorityClient(sapb.NewStorageAuthorityClient(saConn))
211-
212212
// TODO(patf): remove once RA.authorizationLifetimeDays is deployed
213213
authorizationLifetime := 300 * 24 * time.Hour
214214
if c.RA.AuthorizationLifetimeDays != 0 {

‎cmd/boulder-sa/main.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,7 @@ func main() {
115115
serverMetrics := bgrpc.NewServerMetrics(scope)
116116
grpcSrv, listener, err := bgrpc.NewServer(c.SA.GRPC, tls, serverMetrics, clk, bgrpc.NoCancelInterceptor)
117117
cmd.FailOnError(err, "Unable to setup SA gRPC server")
118-
gw := bgrpc.NewStorageAuthorityServer(sai)
119-
sapb.RegisterStorageAuthorityServer(grpcSrv, gw)
118+
sapb.RegisterStorageAuthorityServer(grpcSrv, sai)
120119
hs := health.NewServer()
121120
healthpb.RegisterHealthServer(grpcSrv, hs)
122121

‎cmd/boulder-wfe/main.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ type config struct {
8080
}
8181
}
8282

83-
func setupWFE(c config, logger blog.Logger, stats prometheus.Registerer, clk clock.Clock) (rapb.RegistrationAuthorityClient, core.StorageAuthority, noncepb.NonceServiceClient, map[string]noncepb.NonceServiceClient) {
83+
func setupWFE(c config, logger blog.Logger, stats prometheus.Registerer, clk clock.Clock) (rapb.RegistrationAuthorityClient, sapb.StorageAuthorityClient, noncepb.NonceServiceClient, map[string]noncepb.NonceServiceClient) {
8484
tlsConfig, err := c.WFE.TLS.Load()
8585
cmd.FailOnError(err, "TLS config")
8686

@@ -91,7 +91,7 @@ func setupWFE(c config, logger blog.Logger, stats prometheus.Registerer, clk clo
9191

9292
saConn, err := bgrpc.ClientSetup(c.WFE.SAService, tlsConfig, clientMetrics, clk, grpc.CancelTo408Interceptor)
9393
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
94-
sac := bgrpc.NewStorageAuthorityClient(sapb.NewStorageAuthorityClient(saConn))
94+
sac := sapb.NewStorageAuthorityClient(saConn)
9595

9696
var rns noncepb.NonceServiceClient
9797
npm := map[string]noncepb.NonceServiceClient{}

‎cmd/boulder-wfe2/main.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/honeycombio/beeline-go"
1717
"github.com/jmhodges/clock"
1818
"github.com/letsencrypt/boulder/cmd"
19-
"github.com/letsencrypt/boulder/core"
2019
"github.com/letsencrypt/boulder/features"
2120
"github.com/letsencrypt/boulder/goodkey"
2221
"github.com/letsencrypt/boulder/grpc"
@@ -267,7 +266,7 @@ func loadChain(certFiles []string) (*issuance.Certificate, []byte, error) {
267266
return certs[0], buf.Bytes(), nil
268267
}
269268

270-
func setupWFE(c config, logger blog.Logger, stats prometheus.Registerer, clk clock.Clock) (rapb.RegistrationAuthorityClient, core.StorageAuthority, noncepb.NonceServiceClient, map[string]noncepb.NonceServiceClient) {
269+
func setupWFE(c config, logger blog.Logger, stats prometheus.Registerer, clk clock.Clock) (rapb.RegistrationAuthorityClient, sapb.StorageAuthorityClient, noncepb.NonceServiceClient, map[string]noncepb.NonceServiceClient) {
271270
tlsConfig, err := c.WFE.TLS.Load()
272271
cmd.FailOnError(err, "TLS config")
273272
clientMetrics := bgrpc.NewClientMetrics(stats)
@@ -277,7 +276,7 @@ func setupWFE(c config, logger blog.Logger, stats prometheus.Registerer, clk clo
277276

278277
saConn, err := bgrpc.ClientSetup(c.WFE.SAService, tlsConfig, clientMetrics, clk, grpc.CancelTo408Interceptor)
279278
cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
280-
sac := bgrpc.NewStorageAuthorityClient(sapb.NewStorageAuthorityClient(saConn))
279+
sac := sapb.NewStorageAuthorityClient(saConn)
281280

282281
var rns noncepb.NonceServiceClient
283282
npm := map[string]noncepb.NonceServiceClient{}

0 commit comments

Comments
 (0)
X Tutup