Support more recent protoc-gen-go

[raboof]

When building the project with protoc-gen-go version 1.5.1,
it fails with the following:

protoc -I. ui.proto --go_out=plugins=grpc:../daemon/ui/protocol/
protoc-gen-go: unable to determine Go import path for "ui.proto"

Please specify either:
	• a "go_package" option in the .proto source file, or
	• a "M" argument on the command line.

See https://developers.google.com/protocol-buffers/docs/reference/go-generated#package for more information.

--go_out: protoc-gen-go: Plugin failed with status code 1.

This can be fixed by adding the full go package as an option in the
proto file. To make sure the code is generated to the correct path,
we also have to add add the paths=source_relative option to the
protoc plugin.

After this, the code is generated correctly, but the generated code
references classes like grpc.ClientConnInterface which were introduced
in 1.27.0.

[themighty1]

Thanks, this worked for me. Made the ui/protocol/ui.pb.go:1464:7: undefined: grpc.ClientConnInterface error go away.

[themighty1]

@gustavo-iniguez-goya , I think we're gonna need this to get rid of CI builds errors.

[gustavo-iniguez-goya]

I was more concerned about the go.mod changes.

until now we've been compatible with the no-pip-dependencies rpm package from >= fedora29. But now that we depend on grpc >= 1.27 maybe we're even incompatible with f32:
* Thu Mar 25 2021 Benjamin A. Beasley <code@musicinmybrain.net> - 1.26.0-14
https://koji.fedoraproject.org/koji/buildinfo?buildID=1728134

I haven't had time to verify it, but I'll do it this week.

[themighty1]

How about we keep the grpc version as it is now and incorporate this bash script #359 (comment)
into the Makefile ?

[raboof]

But now that we depend on grpc >= 1.27 maybe we're even incompatible with f32

Good point. If we care about particular distro's perhaps we should try and add them to CI? Do we care about f32? https://fedorapeople.org/groups/schedule/f-34/f-34-key-tasks.html suggests it's going end-of-life next month, so people still using it might be happy to also use an older version of opensnitch?

https://koji.fedoraproject.org/koji/buildinfo?buildID=1728134

Note I don't see go mentioned in that package. It might be that having a minor version difference between the go library and the python library does not lead to problems? After all, the gRPC wire format is rather stable.

I tried the following:

$ docker run -ti fedora:32
# yum install git make protobuf protobuf-devel python3-grpcio golang
# go get google.golang.org/protobuf/cmd/protoc-gen-go \
         google.golang.org/grpc/cmd/protoc-gen-go-grpc
# export PATH="$PATH:$(go env GOPATH)/bin"
# git clone https://github.com/evilsocket/opensnitch
# cd opensnitch
# make

This leads to the following error:

make[1]: Entering directory '/opensnitch/proto'
protoc -I. ui.proto --go_out=plugins=grpc:../daemon/ui/protocol/ --go_opt=paths=source_relative
--go_out: protoc-gen-go: plugins are not supported; use 'protoc --go-grpc_out=...' to generate gRPC

See https://grpc.io/docs/languages/go/quickstart/#regenerate-grpc-code for more information.

I changed the proto/Makefile to look like:

protoc -I. ui.proto --go-grpc_out=paths=source_relative:../daemon/ui/protocol/

Now daemon/ui/protocol/ui_grpc.pb.go seems to be created successfully.

There's another problem with the python code generation, though: make now fails with:

protoc -I. ui.proto --go-grpc_out=paths=source_relative:../daemon/ui/protocol/
python3 -m grpc_tools.protoc -I. --python_out=../ui/opensnitch/ --grpc_python_out=../ui/opensnitch/ ui.proto
/usr/bin/python3: Error while finding module specification for 'grpc_tools.protoc' (ModuleNotFoundError: No module named 'grpc_tools')
make[1]: *** [Makefile:7: ../ui/opensnitch/ui_pb2.py] Error 1

That code seems to have been like that for ages though. Did that ever work on f32?

[gustavo-iniguez-goya]

Good point. If we care about particular distro's perhaps we should try and add them to CI? Do we care about f32?

actually no, I don't care about f32 in particular, but all the distributions. However the changes you're proposing are for NixOS or you particular setup (from another PR: My OS comes with grpcio-tools 1.36.1, which seems to work fine).

(...) suggests it's going end-of-life next month

people keeps using out-of-date distributions and software, like ubuntu 18.04, ubuntu16 (#344), mint19 .. (take a look at the closed issues). Should we drop support for those distributions? I doubt it, as long as we don't require new library/packages versions for fixing issues, etc., or the changes to make it backwards compatible are minimal (3f3e2d6#diff-0e39c69c5a87dd2c07a91a68bea87fd0b9ccf35c1069eb8985d8680de058375b).

Did that ever work on f32?

yep, in fact from >= f29, #332

[raboof]

(...) suggests it's going end-of-life next month

people keeps using out-of-date distributions and software, like ubuntu 18.04, ubuntu16 (#344), mint19 .. (take a look at the closed issues). Should we drop support for those distributions? I doubt it, as long as we don't require new library/packages versions for fixing issues, etc., or the changes to make it backwards compatible are minimal (3f3e2d6#diff-0e39c69c5a87dd2c07a91a68bea87fd0b9ccf35c1069eb8985d8680de058375b).

I completely agree that if we can make things work with both new and old versions that's preferable.

Did that ever work on f32?

yep, in fact from >= f29, #332

That does not prove that python3 -m grpc_tools.protoc -I. --python_out=../ui/opensnitch/ --grpc_python_out=../ui/opensnitch/ ui.proto ever worked on f32, though.

That issue is from january, which is before #346. Before #346 we were checking in ui/opensnitch/ui_pb2.py so people building opensnitch didn't need to generate it themselves. So it seems we might have broken building on f32 with that change already...

[gustavo-iniguez-goya]

That does not prove that python3 -m grpc_tools.protoc -I. --python_out=../ui/opensnitch/ --grpc_python_out=../ui/opensnitch/ ui.proto ever worked on f32, though.
That issue is from january, which is before #346. Before #346 we were checking in ui/opensnitch/ui_pb2.py so people building opensnitch didn't need to generate it themselves. So it seems we might have broken building on f32 with that change already...

Well, the only way to prove it is you to checkout the code and see if it builds or not. I never had a problem generating python's grpc code, only Go's. But the fact is it worked for sebastian and me, and now we can't provide a rpm package without pip dependencies.

Another side effect is that existing installations won't have grpcio-tools 1.36.1 in many cases (as I've just tested on my kubuntu 20.04), so we'll need to upgrade it in future versions if we don't want to cause troubles.

By the way, kubuntu 20.04 by default has python3-grpcio 1.16.1 in the repositories, making us incompatible with this change if the user wanted to use that package instead of pip3's. Also now we can't provide deb packages with python3-grpcio dependency for this reason (debian sid: 1.30.2-3, buster: 1.16.1-1, etc). Yes, well, we can, but the number of compatible distros will be minimal as of today.

I'm just listing the issues I'm running into as I test it. I have to take them into account.

[raboof]

That does not prove that python3 -m grpc_tools.protoc -I. --python_out=../ui/opensnitch/ --grpc_python_out=../ui/opensnitch/ ui.proto ever worked on f32, though

Well, the only way to prove it is you to checkout the code and see if it builds or not

It didn't work for me even before merging this PR:

$ docker run -ti fedora:32
# yum install git make protobuf protobuf-devel python3-grpcio golang
# go get google.golang.org/protobuf/cmd/protoc-gen-go \
         google.golang.org/grpc/cmd/protoc-gen-go-grpc
# export PATH="$PATH:$(go env GOPATH)/bin"
# git clone https://github.com/evilsocket/opensnitch
# cd opensnitch
# git checkout 148526e^
# cd proto
# python3 -m grpc_tools.protoc -I. --python_out=../ui/opensnitch/ --grpc_python_out=../ui/opensnitch/ ui.proto
/usr/bin/python3: Error while finding module specification for 'grpc_tools.protoc' (ModuleNotFoundError: No module named 'grpc_tools')

Did you install grpc_tools on f32? How?

the fact is it worked for sebastian and me, and now we can't provide a rpm package without pip dependencies.

I agree it doesn't work, but you seem to be saying it was broken by this PR. That doesn't make sense - this PR doesn't touch the python part at all.

Another side effect is that existing installations won't have grpcio-tools 1.36.1 in many cases (as I've just tested on my kubuntu 20.04), so we'll need to upgrade it in future versions if we don't want to cause troubles

By the way, kubuntu 20.04 by default has python3-grpcio 1.16.1 in the repositories, making us incompatible with this change if the user wanted to use that package instead of pip3's. Also now we can't provide deb packages with python3-grpcio dependency for this reason (debian sid: 1.30.2-3, buster: 1.16.1-1, etc). Yes, well, we can, but the number of compatible distros will be minimal as of today.

I don't understand what this has to do with this PR - this PR only touched the Go generation, not the python part, right?

It sounds like you are talking about #383, is that correct? I'm definitely not proposing we should require grpcio-tools 1.36.1, and that's not what that PR is doing either...