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.

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

ttnpb imports invalid version of grpc-gateway #5986

Closed
2 of 4 tasks
jpmeijers opened this issue Dec 20, 2022 · 5 comments
Closed
2 of 4 tasks

ttnpb imports invalid version of grpc-gateway #5986

jpmeijers opened this issue Dec 20, 2022 · 5 comments
Assignees
Labels
technical debt Not necessarily broken, but could be done better/cleaner
Milestone

Comments

@jpmeijers
Copy link

jpmeijers commented Dec 20, 2022

Summary

When one imports the golang types from ttnpb into a project, there is an error with the required grpc-gateway version.

One can work around this issue by overriding the grpc-gateway version in your go.sum file:

// But the original grpc-gateway v2.
replace github.com/grpc-ecosystem/grpc-gateway/v2 => github.com/grpc-ecosystem/grpc-gateway/v2 v2.10.3

Steps to Reproduce

  1. Add imports to go program:
	"go.thethings.network/lorawan-stack/v3/pkg/jsonpb"
	"go.thethings.network/lorawan-stack/v3/pkg/ttnpb"
  1. Try to unmarshal an uplink message:
func (handlerContext *Context) PostV3Uplink(w http.ResponseWriter, r *http.Request) {

	body, err := ioutil.ReadAll(r.Body)

	var packetIn ttnpb.ApplicationUp

	contentType := r.Header.Get("Content-Type")
	if contentType == "application/json" {
		marshaler := jsonpb.TTN()
		if err := marshaler.Unmarshal(body, &packetIn); err != nil {
			// Can not parse json body
		}
	} else if contentType == "application/protobuf" || contentType == "application/x-protobuf" || contentType == "application/octet-stream" { // TTS uses application/octet-stream
		if err := proto.Unmarshal(body, &packetIn); err != nil {
			// Can not parse protobuf body
		}
	}

Current Result

	go.thethings.network/lorawan-stack/v3/pkg/ttnpb imports
	github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options: github.com/grpc-ecosystem/grpc-gateway/[email protected]: invalid version: unknown revision 000000000000

Expected Result

One should be able to import go.thethings.network/lorawan-stack/v3/pkg/ttnpb without any special overrides.

Relevant Logs

No response

URL

No response

Deployment

The Things Stack Community Edition

The Things Stack Version

3.23.1

Client Name and Version

No response

Other Information

No response

Proposed Fix

Use grpc-gateway release from upstream.

Contributing

  • I can help by doing more research.
  • I can help by implementing a fix after the proposal above is approved.
  • I can help by testing the fix before it's released.

Code of Conduct

@jpmeijers jpmeijers added the needs/triage We still need to triage this label Dec 20, 2022
@jpmeijers
Copy link
Author

Using the workaround mentioned, I'm getting another error:

go: gopkg.in/DATA-DOG/[email protected]: invalid version: unknown revision 000000000000

Which can be worked around be adding the following to go.sum:

replace gopkg.in/DATA-DOG/go-sqlmock.v1 => gopkg.in/DATA-DOG/go-sqlmock.v1 v1.3.0

@adriansmares adriansmares added the technical debt Not necessarily broken, but could be done better/cleaner label Dec 23, 2022
@adriansmares adriansmares added this to the 2023 Q1 milestone Dec 23, 2022
@adriansmares adriansmares self-assigned this Dec 23, 2022
@adriansmares
Copy link
Contributor

References #2798.

Indeed, we're currently using a lot of replacements due to our gogoproto dependency. We'll hopefully be able to get rid of the ones mentioned here and a lot more once we finish removing gogoproto.

@adriansmares adriansmares removed the needs/triage We still need to triage this label Dec 23, 2022
@adriansmares
Copy link
Contributor

The v3.25 branch now contains the gogoproto-less version of the stack. It has a lot less replacements and lots of dependencies have been updated. Only the throttled replacement will be needed for now.

While the upgrade simplifies the dependencies significantly, it is not fully in place. The move away from gogoproto to vanilla goproto caused the standard type wrappers (timestamps, durations, structs) to be changed to their vanilla counter part. In practice this means that after the upgrade, places that used types.Struct or types.Timestamp will need to become structpb.Struct and timestamppb.Timestamp - it's mainly a find and replace job under VS Code.

Examples of how this mind numbing find and replace went can be found in the commits at #6032 - there is a commit for each type.

@jpmeijers
Copy link
Author

jpmeijers commented Feb 10, 2023

I updated my project using:
go get go.thethings.network/lorawan-stack/v3/pkg/ttnpb@02d6710
go get go.thethings.network/lorawan-stack/v3/pkg/jsonpb@02d6710

Getting this warning:
go: module github.com/golang/protobuf is deprecated: Use the "google.golang.org/protobuf" module instead.

The only override in my go.mod is:
replace github.com/throttled/throttled/v2 => github.com/TheThingsIndustries/throttled/v2 v2.7.1-noredis

I only tested unmarshalling JSON, but that works as expected. I'll update another stack that uses protobufs for the webhook and confirm that works too.

Update:
image
I can confirm my Go backend is unmarshalling the Protocol Buffer webhook uplinks correctly.

@adriansmares
Copy link
Contributor

Closing this issue as this individual error is no longer relevant on the current head branch. The clean protobuf publication discussion is to follow in the linked issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical debt Not necessarily broken, but could be done better/cleaner
Projects
None yet
Development

No branches or pull requests

2 participants