Remove ca gRPC wrapper (#5330)

Delete the CertificateAuthorityClientWrapper, OCSPGeneratorClientWrapper, and CertificateAuthorityServerWrapper structs, which provided no error checking above and beyond their wrapped types. Replace them with the corresponding auto-generated gRPC types in calling code. Update some mocks to have the necessary variadic grpc.CallOption parameter. Finally, delete the now-unused core.CertificateAuthority interface. Fixes #5324
letsencrypt · Mar 11, 2021 · 993953b · 993953b
1 parent de8b560
commit 993953b
Show file tree

Hide file tree

Showing 9 changed files with 23 additions and 105 deletions.
diff --git a/cmd/admin-revoker/main_test.go b/cmd/admin-revoker/main_test.go
@@ -26,13 +26,14 @@ import (
 	"github.com/letsencrypt/boulder/sa/satest"
 	"github.com/letsencrypt/boulder/test"
 	"github.com/letsencrypt/boulder/test/vars"
+	"google.golang.org/grpc"
 )
 
 type mockCA struct {
 	mocks.MockCA
 }
 
-func (ca *mockCA) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {
+func (ca *mockCA) GenerateOCSP(context.Context, *capb.GenerateOCSPRequest, ...grpc.CallOption) (*capb.OCSPResponse, error) {
 	return &capb.OCSPResponse{}, nil
 }
 

diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go
@@ -348,8 +348,7 @@ func main() {
 
 	caSrv, caListener, err := bgrpc.NewServer(c.CA.GRPCCA, tlsConfig, serverMetrics, clk)
 	cmd.FailOnError(err, "Unable to setup CA gRPC server")
-	caWrapper := bgrpc.NewCertificateAuthorityServer(cai)
-	capb.RegisterCertificateAuthorityServer(caSrv, caWrapper)
+	capb.RegisterCertificateAuthorityServer(caSrv, cai)
 	caHealth := health.NewServer()
 	healthpb.RegisterHealthServer(caSrv, caHealth)
 	wg.Add(1)
@@ -360,8 +359,7 @@ func main() {
 
 	ocspSrv, ocspListener, err := bgrpc.NewServer(c.CA.GRPCOCSPGenerator, tlsConfig, serverMetrics, clk)
 	cmd.FailOnError(err, "Unable to setup CA gRPC server")
-	ocspWrapper := bgrpc.NewCertificateAuthorityServer(cai)
-	capb.RegisterOCSPGeneratorServer(ocspSrv, ocspWrapper)
+	capb.RegisterOCSPGeneratorServer(ocspSrv, cai)
 	ocspHealth := health.NewServer()
 	healthpb.RegisterHealthServer(ocspSrv, ocspHealth)
 	wg.Add(1)

diff --git a/cmd/boulder-ra/main.go b/cmd/boulder-ra/main.go
@@ -155,7 +155,7 @@ func main() {
 
 	caConn, err := bgrpc.ClientSetup(c.RA.CAService, tlsConfig, clientMetrics, clk)
 	cmd.FailOnError(err, "Unable to create CA client")
-	cac := bgrpc.NewCertificateAuthorityClient(capb.NewCertificateAuthorityClient(caConn))
+	cac := capb.NewCertificateAuthorityClient(caConn)
 
 	var ctp *ctpolicy.CTPolicy
 	conn, err := bgrpc.ClientSetup(c.RA.PublisherService, tlsConfig, clientMetrics, clk)

diff --git a/cmd/ocsp-updater/main.go b/cmd/ocsp-updater/main.go
@@ -370,7 +370,7 @@ func main() {
 	clientMetrics := bgrpc.NewClientMetrics(stats)
 	caConn, err := bgrpc.ClientSetup(c.OCSPUpdater.OCSPGeneratorService, tlsConfig, clientMetrics, clk)
 	cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to CA")
-	ogc := bgrpc.NewOCSPGeneratorClient(capb.NewOCSPGeneratorClient(caConn))
+	ogc := capb.NewOCSPGeneratorClient(caConn)
 
 	updater, err := newUpdater(
 		stats,

diff --git a/core/interfaces.go b/core/interfaces.go
@@ -9,7 +9,6 @@ import (
 
 	jose "gopkg.in/square/go-jose.v2"
 
-	capb "github.com/letsencrypt/boulder/ca/proto"
 	corepb "github.com/letsencrypt/boulder/core/proto"
 	"github.com/letsencrypt/boulder/identifier"
 	pubpb "github.com/letsencrypt/boulder/publisher/proto"
@@ -96,17 +95,6 @@ type RegistrationAuthority interface {
 // TODO(#4956): Remove this unnecessary type alias.
 type ValidationAuthority vapb.VAServer
 
-// CertificateAuthority defines the public interface for the Boulder CA
-type CertificateAuthority interface {
-	// [RegistrationAuthority]
-	IssuePrecertificate(ctx context.Context, issueReq *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error)
-
-	// [RegistrationAuthority]
-	IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error)
-
-	GenerateOCSP(ctx context.Context, ocspReq *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error)
-}
-
 // PolicyAuthority defines the public interface for the Boulder PA
 type PolicyAuthority interface {
 	WillingToIssue(domain identifier.ACMEIdentifier) error

diff --git a/grpc/ca-wrappers.go b/grpc/ca-wrappers.go
diff --git a/mocks/ca.go b/mocks/ca.go
@@ -8,7 +8,7 @@ import (
 
 	capb "github.com/letsencrypt/boulder/ca/proto"
 	corepb "github.com/letsencrypt/boulder/core/proto"
-	"github.com/letsencrypt/boulder/revocation"
+	"google.golang.org/grpc"
 )
 
 // MockCA is a mock of a CA that always returns the cert from PEM in response to
@@ -18,7 +18,7 @@ type MockCA struct {
 }
 
 // IssuePrecertificate is a mock
-func (ca *MockCA) IssuePrecertificate(ctx context.Context, _ *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error) {
+func (ca *MockCA) IssuePrecertificate(ctx context.Context, _ *capb.IssueCertificateRequest, _ ...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) {
 	if ca.PEM == nil {
 		return nil, fmt.Errorf("MockCA's PEM field must be set before calling IssueCertificate")
 	}
@@ -33,7 +33,7 @@ func (ca *MockCA) IssuePrecertificate(ctx context.Context, _ *capb.IssueCertific
 }
 
 // IssueCertificateForPrecertificate is a mock
-func (ca *MockCA) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
+func (ca *MockCA) IssueCertificateForPrecertificate(ctx context.Context, req *capb.IssueCertificateForPrecertificateRequest, _ ...grpc.CallOption) (*corepb.Certificate, error) {
 	return &corepb.Certificate{
 		Der:            req.DER,
 		RegistrationID: 1,
@@ -45,11 +45,6 @@ func (ca *MockCA) IssueCertificateForPrecertificate(ctx context.Context, req *ca
 }
 
 // GenerateOCSP is a mock
-func (ca *MockCA) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {
+func (ca *MockCA) GenerateOCSP(ctx context.Context, req *capb.GenerateOCSPRequest, _ ...grpc.CallOption) (*capb.OCSPResponse, error) {
 	return nil, nil
 }
-
-// RevokeCertificate is a mock
-func (ca *MockCA) RevokeCertificate(ctx context.Context, serial string, reasonCode revocation.Reason) (err error) {
-	return
-}
diff --git a/ra/ra.go b/ra/ra.go
@@ -58,7 +58,7 @@ type caaChecker interface {
 // NOTE: All of the fields in RegistrationAuthorityImpl need to be
 // populated, or there is a risk of panic.
 type RegistrationAuthorityImpl struct {
-	CA        core.CertificateAuthority
+	CA        capb.CertificateAuthorityClient
 	VA        vapb.VAClient
 	SA        core.StorageAuthority
 	PA        core.PolicyAuthority

diff --git a/ra/ra_test.go b/ra/ra_test.go
@@ -3526,8 +3526,9 @@ type mockCAFailPrecert struct {
 }
 
 func (ca *mockCAFailPrecert) IssuePrecertificate(
-	_ context.Context,
-	_ *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error) {
+	context.Context,
+	*capb.IssueCertificateRequest,
+	...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) {
 	return nil, ca.err
 }
 
@@ -3539,7 +3540,10 @@ type mockCAFailCertForPrecert struct {
 }
 
 // IssuePrecertificate needs to be mocked for mockCAFailCertForPrecert's `IssueCertificateForPrecertificate` to get called.
-func (ca *mockCAFailCertForPrecert) IssuePrecertificate(_ context.Context, _ *capb.IssueCertificateRequest) (*capb.IssuePrecertificateResponse, error) {
+func (ca *mockCAFailCertForPrecert) IssuePrecertificate(
+	context.Context,
+	*capb.IssueCertificateRequest,
+	...grpc.CallOption) (*capb.IssuePrecertificateResponse, error) {
 	k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
 	if err != nil {
 		return nil, err
@@ -3564,8 +3568,9 @@ func (ca *mockCAFailCertForPrecert) IssuePrecertificate(_ context.Context, _ *ca
 }
 
 func (ca *mockCAFailCertForPrecert) IssueCertificateForPrecertificate(
-	_ context.Context,
-	_ *capb.IssueCertificateForPrecertificateRequest) (*corepb.Certificate, error) {
+	context.Context,
+	*capb.IssueCertificateForPrecertificateRequest,
+	...grpc.CallOption) (*corepb.Certificate, error) {
 	return &corepb.Certificate{}, ca.err
 }
 
@@ -3652,7 +3657,7 @@ func TestIssueCertificateInnerErrs(t *testing.T) {
 
 	testCases := []struct {
 		Name         string
-		Mock         core.CertificateAuthority
+		Mock         capb.CertificateAuthorityClient
 		ExpectedErr  error
 		ExpectedProb *berrors.BoulderError
 	}{
@@ -3852,7 +3857,7 @@ type mockCAOCSP struct {
 	mocks.MockCA
 }
 
-func (mcao *mockCAOCSP) GenerateOCSP(context.Context, *capb.GenerateOCSPRequest) (*capb.OCSPResponse, error) {
+func (mcao *mockCAOCSP) GenerateOCSP(context.Context, *capb.GenerateOCSPRequest, ...grpc.CallOption) (*capb.OCSPResponse, error) {
 	return &capb.OCSPResponse{Response: []byte{1, 2, 3}}, nil
 }