Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gRPC: fix server reflection #8945

Merged
merged 18 commits into from
Mar 22, 2021
Merged

gRPC: fix server reflection #8945

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d4db860
add: gogoreflection
fdymylja Mar 19, 2021
5c11486
fix: server reflection
fdymylja Mar 19, 2021
034b0d8
fix: make tests resolve imports in a transitive way
fdymylja Mar 19, 2021
cb21912
chore: cleanup getMessageType
fdymylja Mar 19, 2021
9b2800f
Merge branch 'master' into fdymylja/issue-8927-fix-grpc-reflection
fdymylja Mar 19, 2021
0d82b78
Merge branch 'master' into fdymylja/issue-8927-fix-grpc-reflection
fdymylja Mar 22, 2021
79d481e
chore: lint
fdymylja Mar 22, 2021
5875f01
Merge remote-tracking branch 'origin/fdymylja/issue-8927-fix-grpc-ref…
fdymylja Mar 22, 2021
d1a1ff9
Merge branch 'master' into fdymylja/issue-8927-fix-grpc-reflection
jgimeno Mar 22, 2021
6567491
add: extensions reflection
fdymylja Mar 22, 2021
716a145
chore: update interact-node.md docs
fdymylja Mar 22, 2021
03f2c8d
Merge remote-tracking branch 'origin/fdymylja/issue-8927-fix-grpc-ref…
fdymylja Mar 22, 2021
ed0cd48
Merge branch 'master' into fdymylja/issue-8927-fix-grpc-reflection
Mar 22, 2021
b7c26e4
chore: update CHANGELOG.md
fdymylja Mar 22, 2021
a0a7f84
Merge branch 'master' into fdymylja/issue-8927-fix-grpc-reflection
fdymylja Mar 22, 2021
ba4698c
Merge branch 'master' into fdymylja/issue-8927-fix-grpc-reflection
Mar 22, 2021
b26b502
Merge branch 'master' into fdymylja/issue-8927-fix-grpc-reflection
Mar 22, 2021
def738b
Update server/grpc/gogoreflection/fix_registration.go
fdymylja Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ require (
github.com/hashicorp/golang-lru v0.5.4
github.com/hdevalence/ed25519consensus v0.0.0-20210204194344-59a8610d2b87
github.com/improbable-eng/grpc-web v0.14.0
github.com/jhump/protoreflect v1.8.2
github.com/magiconair/properties v1.8.4
github.com/mattn/go-isatty v0.0.12
github.com/otiai10/copy v1.5.0
Expand All @@ -53,7 +54,7 @@ require (
golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad
google.golang.org/genproto v0.0.0-20210114201628-6edceaf6022f
google.golang.org/grpc v1.36.0
google.golang.org/protobuf v1.25.0
google.golang.org/protobuf v1.25.1-0.20200805231151-a709e31e5d12
gopkg.in/ini.v1 v1.61.0 // indirect
gopkg.in/yaml.v2 v2.4.0
nhooyr.io/websocket v1.8.6 // indirect
Expand Down
11 changes: 11 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+
github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1 h1:EGx4pi6eqNxGaHF6qqu48+N2wcFQ5qg5FXgOdqsJ5d8=
github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/gordonklaus/ineffassign v0.0.0-20200309095847-7953dde2c7bf/go.mod h1:cuNKsD1zp2v6XfE/orVX2QE1LC+i254ceGcVeDT3pTU=
github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
github.com/gorilla/handlers v1.5.1 h1:9lRY6j8DEeeBT10CvO9hGW0gmky0BprnvDI5vfhUHH4=
github.com/gorilla/handlers v1.5.1/go.mod h1:t8XrUpc4KVXb7HGyJ4/cEnwQiaxrX/hz1Zv/4g96P1Q=
Expand Down Expand Up @@ -368,6 +369,8 @@ github.com/influxdata/influxdb1-client v0.0.0-20191209144304-8bf82d3c094d/go.mod
github.com/jackpal/go-nat-pmp v1.0.2-0.20160603034137-1fa385a6f458/go.mod h1:QPH045xvCAeXUZOxsnwmrtiCoxIr9eob+4orBN1SBKc=
github.com/jessevdk/go-flags v0.0.0-20141203071132-1679536dcc89/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/jhump/protoreflect v1.8.2 h1:k2xE7wcUomeqwY0LDCYA16y4WWfyTcMx5mKhk0d4ua0=
github.com/jhump/protoreflect v1.8.2/go.mod h1:7GcYQDdMU/O/BBrl/cX6PNHpXh6cenjd8pneu5yW7Tg=
github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
github.com/jmhodges/levigo v1.0.0 h1:q5EC36kV79HWeTBWsod3mG11EgStG3qArTKcvlksN1U=
github.com/jmhodges/levigo v1.0.0/go.mod h1:Q6Qx+uH3RAqyK4rFQroq9RL7mdkABMcfhEI+nNuzMJQ=
Expand Down Expand Up @@ -472,6 +475,7 @@ github.com/nats-io/nkeys v0.1.3/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxzi
github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
github.com/nishanths/predeclared v0.0.0-20200524104333-86fad755b4d3/go.mod h1:nt3d53pc1VYcphSCIaYAJtnPYnr3Zyn8fMq2wvPGPso=
github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
github.com/oklog/oklog v0.3.2/go.mod h1:FCV+B7mhrz4o+ueLpx+KqkyXRGMWOYEvfiXtdGtbWGs=
Expand Down Expand Up @@ -697,6 +701,7 @@ github.com/wsddn/go-ecdh v0.0.0-20161211032359-48726bab9208/go.mod h1:IotVbo4F+m
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/zondax/hid v0.9.0 h1:eiT3P6vNxAEVxXMw66eZUAAnU2zD33JBkfG/EnfAKl8=
github.com/zondax/hid v0.9.0/go.mod h1:l5wttcP0jwtdLjqjMMWFVEE7d1zO0jvSPA9OPZxWpEM=
Expand Down Expand Up @@ -884,10 +889,13 @@ golang.org/x/tools v0.0.0-20191029041327-9cc4af7d6b2c/go.mod h1:b+2E5dAYhXwXZwtn
golang.org/x/tools v0.0.0-20191029190741-b9c20aec41a5/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20191112195655-aa38f8e97acc/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20200103221440-774c71fcf114/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.0.0-20200117012304-6edc0a871e69/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200717024301-6ddee64345a6/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
Expand Down Expand Up @@ -937,6 +945,8 @@ google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpAD
google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4=
google.golang.org/protobuf v1.25.0 h1:Ejskq+SyPohKW+1uil0JJMtmHCgJPJ/qWTxr8qp+R4c=
google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
google.golang.org/protobuf v1.25.1-0.20200805231151-a709e31e5d12 h1:OwhZOOMuf7leLaSCuxtQ9FW7ui2L2L6UKOtKAUqovUQ=
google.golang.org/protobuf v1.25.1-0.20200805231151-a709e31e5d12/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down Expand Up @@ -977,6 +987,7 @@ honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWh
honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
nhooyr.io/websocket v1.8.6 h1:s+C3xAMLwGmlI31Nyn/eAehUlZPwfYZu2JXM621Q5/k=
nhooyr.io/websocket v1.8.6/go.mod h1:B70DZP8IakI65RVQ51MsWP/8jndNma26DVA/nFSCgW0=
rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
Expand Down
5 changes: 5 additions & 0 deletions server/grpc/gogoreflection/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Package gogoreflection implements gRPC reflection for gogoproto consumers
// the normal reflection library does not work as it points to a different
// singleton registry. The API and codebase is taken from the official gRPC
// reflection repository.
package gogoreflection
100 changes: 100 additions & 0 deletions server/grpc/gogoreflection/fix_registration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package gogoreflection

import (
"bytes"
"compress/gzip"
"fmt"
"reflect"

_ "github.com/gogo/protobuf/gogoproto" // required so it does register the gogoproto file descriptor
gogoproto "github.com/gogo/protobuf/proto"

// nolint: staticcheck
"github.com/golang/protobuf/proto"
dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
fdymylja marked this conversation as resolved.
Show resolved Hide resolved
_ "github.com/regen-network/cosmos-proto" // look above
)

var importsToFix = map[string]string{
"gogo.proto": "gogoproto/gogo.proto",
"cosmos.proto": "cosmos_proto/cosmos.proto",
}

// fixRegistration is required because certain files register themselves in a way
// but are imported by other files in a different way.
fdymylja marked this conversation as resolved.
Show resolved Hide resolved
// NOTE(fdymylja): This fix should not be needed and should be addressed in some CI.
// Currently every cosmos-sdk proto file is importing gogo.proto as gogoproto/gogo.proto,
// but gogo.proto registers itself as gogo.proto, same goes for cosmos.proto.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why this happens? Maybe we should fix it in github.com/regen-network/cosmos-proto?
@aaronc

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean - let's create an issue if there is a bug we can easily fix in cosmos-proto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not a bug per se, it's a shortcoming when it comes to combo together reflection and proto imports. protofiles register themselve in the registry with a path, but then are imported with another path. this makes reflection fail, because it cannot find certain files.

this would mean either forking gogoproto and changing how we feed the file to protoc, or changing the third_party structure in the sdk, edit how every file imports gogoproto.

and im not sure if this is a breaking change, descriptor wise it is.

probably this should be a blob of work which should be reorganized during the transition to protov2

func fixRegistration(registeredAs, importedAs string) error {
raw := gogoproto.FileDescriptor(registeredAs)
if len(raw) == 0 {
return fmt.Errorf("file descriptor not found: %s", registeredAs)
fdymylja marked this conversation as resolved.
Show resolved Hide resolved
}

fd, err := decodeFileDesc(raw)
if err != nil {
return err
}

// fix name
*fd.Name = importedAs
fixedRaw, err := compress(fd)
if err != nil {
return fmt.Errorf("unable to compress: %w", err)
}
gogoproto.RegisterFile(importedAs, fixedRaw)
return nil
}

func init() {
// we need to fix the gogoproto filedesc to match the import path
// in theory this shouldn't be required, generally speaking
// proto files should be imported as their registration path

for registeredAs, importedAs := range importsToFix {
err := fixRegistration(registeredAs, importedAs)
if err != nil {
panic(err)
}
}
}

// compress compresses the given file descriptor
// nolint: interfacer
func compress(fd *dpb.FileDescriptorProto) ([]byte, error) {
fdBytes, err := proto.Marshal(fd)
if err != nil {
return nil, err
}
buf := new(bytes.Buffer)
cw := gzip.NewWriter(buf)
_, err = cw.Write(fdBytes)
if err != nil {
return nil, err
}
err = cw.Close()
if err != nil {
return nil, err
}
return buf.Bytes(), nil
}

func getFileDescriptor(filePath string) []byte {
// since we got well known descriptors which are not registered into gogoproto registry
// but are instead registered into the proto one, we need to check both
fd := gogoproto.FileDescriptor(filePath)
if len(fd) != 0 {
return fd
}
// nolint: staticcheck
fdymylja marked this conversation as resolved.
Show resolved Hide resolved
return proto.FileDescriptor(filePath)
}

func getMessageType(name string) reflect.Type {
typ := gogoproto.MessageType(name)
if typ != nil {
return typ
}
// nolint: staticcheck
return proto.MessageType(name)
}
22 changes: 22 additions & 0 deletions server/grpc/gogoreflection/fix_registration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package gogoreflection

import (
"testing"

"google.golang.org/protobuf/runtime/protoimpl"
)

func TestRegistrationFix(t *testing.T) {
res := getFileDescriptor("gogoproto/gogo.proto")
rawDesc, err := decompress(res)
if err != nil {
t.Fatal(err)
}
fd := protoimpl.DescBuilder{
RawDescriptor: rawDesc,
}.Build()

if fd.File.Extensions().Len() == 0 {
t.Fatal("unexpected parsing")
}
}
Loading