Skip to content

Commit

Permalink
Use generic types for gRPC stream implementations (#7501)
Browse files Browse the repository at this point in the history
Update the version of protoc-gen-go-grpc that we use to generate Go gRPC
code from our proto files, and update the versions of other gRPC tools
and libraries that we use to match. Turn on the new
`use_generic_streams` code generation flag to change how
protoc-gen-go-grpc generates implementations of our streaming methods,
from creating a wholly independent implementation for every stream to
using shared generic implementations.

Take advantage of this code-sharing to remove our SA "wrapper" methods,
now that they have truly the same signature as the SARO methods which
they wrap. Also remove all references to the old-style stream names
(e.g. foopb.FooService_BarMethodClient) and replace them with the new
underlying generic names, for the sake of consistency. Finally, also
remove a few custom stream test mocks, replacing them with the generic
mocks.ServerStreamClient.

Note that this PR does not change the names in //mocks/sa.go, to avoid
conflicts with work happening in the pursuit of
#7476. Note also that this
PR updates the version of protoc-gen-go-grpc that we use to a specific
commit. This is because, although a new release of grpc-go itself has
been cut, the codegen binary is a separate Go module with its own
releases, and it hasn't had a new release cut yet. Tracking for that is
in grpc/grpc-go#7030.
  • Loading branch information
aarongable authored May 24, 2024
1 parent 0d8efb9 commit 89213f9
Show file tree
Hide file tree
Showing 72 changed files with 1,090 additions and 1,154 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/boulder-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
matrix:
# Add additional docker image tags here and all tests will be run with the additional image.
BOULDER_TOOLS_TAG:
- go1.22.3_2024-05-13
- go1.22.3_2024-05-22
# Tests command definitions. Use the entire "docker compose" command you want to run.
tests:
# Run ./test.sh --help for a description of each of the flags.
Expand Down
2 changes: 1 addition & 1 deletion akamai/proto/akamai.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 10 additions & 5 deletions akamai/proto/akamai_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion ca/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"io"
"strings"

"google.golang.org/grpc"

capb "github.com/letsencrypt/boulder/ca/proto"
"github.com/letsencrypt/boulder/core"
corepb "github.com/letsencrypt/boulder/core/proto"
Expand Down Expand Up @@ -51,7 +53,7 @@ func NewCRLImpl(
}, nil
}

func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error {
func (ci *crlImpl) GenerateCRL(stream grpc.BidiStreamingServer[capb.GenerateCRLRequest, capb.GenerateCRLResponse]) error {
var issuer *issuance.Issuer
var req *issuance.CRLRequest
rcs := make([]x509.RevocationListEntry, 0)
Expand Down
2 changes: 1 addition & 1 deletion ca/proto/ca.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

95 changes: 37 additions & 58 deletions ca/proto/ca_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 20 additions & 38 deletions cmd/admin/cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"crypto/x509"
"encoding/pem"
"errors"
"io"
"os"
"path"
"slices"
Expand All @@ -25,6 +24,7 @@ import (
corepb "github.com/letsencrypt/boulder/core/proto"
berrors "github.com/letsencrypt/boulder/errors"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/mocks"
rapb "github.com/letsencrypt/boulder/ra/proto"
"github.com/letsencrypt/boulder/revocation"
sapb "github.com/letsencrypt/boulder/sa/proto"
Expand All @@ -41,23 +41,12 @@ type mockSAWithIncident struct {

// SerialsForIncident returns a fake gRPC stream client object which itself
// will return the mockSAWithIncident's serials in order.
func (msa *mockSAWithIncident) SerialsForIncident(_ context.Context, _ *sapb.SerialsForIncidentRequest, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_SerialsForIncidentClient, error) {
return &mockSerialsForIncidentClient{unsentSerials: msa.incidentSerials}, nil
}

type mockSerialsForIncidentClient struct {
grpc.ClientStream
unsentSerials []string
}

// Recv returns the next serial from the pre-loaded list.
func (c *mockSerialsForIncidentClient) Recv() (*sapb.IncidentSerial, error) {
if len(c.unsentSerials) > 0 {
res := c.unsentSerials[0]
c.unsentSerials = c.unsentSerials[1:]
return &sapb.IncidentSerial{Serial: res}, nil
func (msa *mockSAWithIncident) SerialsForIncident(_ context.Context, _ *sapb.SerialsForIncidentRequest, _ ...grpc.CallOption) (grpc.ServerStreamingClient[sapb.IncidentSerial], error) {
fakeResults := make([]*sapb.IncidentSerial, len(msa.incidentSerials))
for i, serial := range msa.incidentSerials {
fakeResults[i] = &sapb.IncidentSerial{Serial: serial}
}
return nil, io.EOF
return &mocks.ServerStreamClient[sapb.IncidentSerial]{Results: fakeResults}, nil
}

func TestSerialsFromIncidentTable(t *testing.T) {
Expand Down Expand Up @@ -99,26 +88,15 @@ type mockSAWithKey struct {

// GetSerialsByKey returns a fake gRPC stream client object which itself
// will return the mockSAWithKey's serials in order.
func (msa *mockSAWithKey) GetSerialsByKey(_ context.Context, req *sapb.SPKIHash, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByKeyClient, error) {
func (msa *mockSAWithKey) GetSerialsByKey(_ context.Context, req *sapb.SPKIHash, _ ...grpc.CallOption) (grpc.ServerStreamingClient[sapb.Serial], error) {
if !slices.Equal(req.KeyHash, msa.keyHash) {
return &mockSerialsClient{}, nil
return &mocks.ServerStreamClient[sapb.Serial]{}, nil
}
return &mockSerialsClient{unsentSerials: msa.serials}, nil
}

type mockSerialsClient struct {
grpc.ClientStream
unsentSerials []string
}

// Recv returns the next serial from the pre-loaded list.
func (c *mockSerialsClient) Recv() (*sapb.Serial, error) {
if len(c.unsentSerials) > 0 {
res := c.unsentSerials[0]
c.unsentSerials = c.unsentSerials[1:]
return &sapb.Serial{Serial: res}, nil
fakeResults := make([]*sapb.Serial, len(msa.serials))
for i, serial := range msa.serials {
fakeResults[i] = &sapb.Serial{Serial: serial}
}
return nil, io.EOF
return &mocks.ServerStreamClient[sapb.Serial]{Results: fakeResults}, nil
}

func TestSerialsFromPrivateKey(t *testing.T) {
Expand Down Expand Up @@ -162,13 +140,17 @@ func (msa *mockSAWithAccount) GetRegistration(_ context.Context, req *sapb.Regis
return &corepb.Registration{}, nil
}

// SerialsForIncident returns a fake gRPC stream client object which itself
// GetSerialsByAccount returns a fake gRPC stream client object which itself
// will return the mockSAWithAccount's serials in order.
func (msa *mockSAWithAccount) GetSerialsByAccount(_ context.Context, req *sapb.RegistrationID, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByAccountClient, error) {
func (msa *mockSAWithAccount) GetSerialsByAccount(_ context.Context, req *sapb.RegistrationID, _ ...grpc.CallOption) (grpc.ServerStreamingClient[sapb.Serial], error) {
if req.Id != msa.regID {
return &mockSerialsClient{}, nil
return &mocks.ServerStreamClient[sapb.Serial]{}, nil
}
fakeResults := make([]*sapb.Serial, len(msa.serials))
for i, serial := range msa.serials {
fakeResults[i] = &sapb.Serial{Serial: serial}
}
return &mockSerialsClient{unsentSerials: msa.serials}, nil
return &mocks.ServerStreamClient[sapb.Serial]{Results: fakeResults}, nil
}

func TestSerialsFromRegID(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/admin/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type mockSARO struct {
sapb.StorageAuthorityReadOnlyClient
}

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

Expand Down
Loading

0 comments on commit 89213f9

Please sign in to comment.