Add protobuf generated code for Go

[asraa]

Signed-off-by: Asra Ali [email protected]

Partial fix for #3 for Go generate code

This adds a CI test and utilities for generated Go protobuf stubs. Please give me suggestions and rip this apart if needed. I don't know:

• Whether the code is best lived alongside the protos
• Whether I should have used go-agnostic tooling to install deps
• Whether a pre-push hook should exist to check generation, or whether CI should do more than just check code generation.

Summary

Release Note

Documentation

[asraa]

something about Go doesn't like the defined go_package names...

[znewman01]

All of the above is purely subjective opinion so feel free to disregard.

Whether the code is best lived alongside the protos

My vote is to pull it out into a top level /gen directory or something

Whether I should have used go-agnostic tooling to install deps

I think that'd be nice. If I'm not a Go developer, I might not have Go installed. (I feel doubly strong about this because Go insists on crapping all over your home directory.) Something Docker based could be cool b/c reproducibility is easier.

Whether a pre-push hook should exist to check generation, or whether CI should do more than just check code generation.

I'd really love to not check in the generated code at all, or possibly to have a separate branch for generated code that only GH Actions can push to. Part of the problem is that GH doesn't have a great story for displaying generated artifacts in the web UI. This is maybe a huge pain for Go, other languages are more reasonable because they have an explicit release process.

[asraa]

Part of the problem is that GH doesn't have a great story for displaying generated artifacts in the web UI.

There's gitattributes which doesn't display the generated code (see this PR too).

My vote is to pull it out into a top level /gen directory or something

Sounds good, I'll move them to generated/

If I'm not a Go developer, I might not have Go installed. (I feel doubly strong about this because Go insists on crapping all over your home directory.)

Lemme see if this can work, protoc can be installed without that, and you just need to go plugin for it. I would want to avoid docker, but that is somewhat nice.

[znewman01]

Part of the problem is that GH doesn't have a great story for displaying generated artifacts in the web UI.

There's gitattributes which doesn't display the generated code (see this PR too).

That's kinda the opposite of what I want.

I'd like the code not to be checked in at all (so we don't run into workflow issues when I forget to do this, and so you don't actually need protoc installed to work on this repo), but I'd still like to be able to look at the generated code (as part of PR reviews and just when browsing repo source). I find that when using generated protobuf code I need to look at the generated code on occasion, so it'd be nice to have that available.

Again, this is me being very particular so feel free to solve the problem in whatever way is expedient right now :)

[kommendorkapten]

Could we switch to containerized generation for the protobuf file? There is a similar issue for Fulcio: sigstore/fulcio#817. When I was playing around with this for the Sigstore bundle, I too used the namely/protoc-all container image, with great success. For me, having a container is easier to work with locally, and it's much easier to get consistent results. I always felt that using the command line protoc is tedious and error-prone.

I'm in favour of checking in the generated code, and have it browsable, but also having this repository being the canonical repository that all clients depend on, to avoid each client having to fiddle with code generation themselves. So this would mean that we have to align the Go modules for the generated code, and prepare a go.mod file, and possibly move the generated code afterwards so the module names aligns with the paths in the directory. The next step would be to prepare generation for other languages and publish the packages to the canonical package repository. But one thing at a time :)

I'd like the code not to be checked in at all (so we don't run into workflow issues when I forget to do this, and so you don't actually need protoc installed to work on this repo), but I'd still like to be able to look at the generated code (as part of PR reviews and just when browsing repo source). I find that when using generated protobuf code I need to look at the generated code on occasion, so it'd be nice to have that available.

I don't really understand this? I'd like the code not to be checked in at all and then but I'd still like to be able to look at the generated code.
At least this: and so you don't actually need protoc installed to work on this repo would be mitigated with a containerized protoc.

[codysoyland]

Just to throw in my 2 cents, I've always found it idiomatic/practical to manage protobuf-generated Go code like this, with a Makefile target and a CI job that recompiles and checks for a diff. The only pain I've had is when protoc versions diverge, but the CI job will make sure that we notice that. I also don't expect contributors to this repo to have any trouble getting protoc installed.

[asraa]

I agree, I also like CI. I'll update to at least (1) move to gen/ and use (2) namely/protoc-all.

I think Zack's was saying he doesn't like being the one to push generated code, but wants CI to be totally responsible for it. I think that becomes a little annoying to mess with, unless we want to do PR reviews for just generated code the github actions bot is performing, or else the generated diffs would be invisible and pushed directly to the branch.

[asraa]

OK final issues is around Go import paths. I think Fredrik has an idea of the best way to do this. I expect I need to change the go_package paths to reference sigstore/protobuf-specs and make the published paths match up. Was there a reason to NOT do this or a workaround to avoid that?

EDIT: maybe I can fiddle with the options like --go-package-map

[znewman01]

I'm willing to concede this one. I'm just philosophically opposed to checking generated code into version control, and I've been burned by forgetting to run make generate about 10,000 times too many. But doing things that are icky but technically work appears to be the Go Way (TM) 😛

+1 to all the other decisions (a gen/ directory, Docker for building)

[codysoyland]

Cool that this works! I think the correct package path is this though:

Suggested change

	docker run -v ${PWD}:/defs ${PROTOC_IMAGE} -i protos -f envelope.proto -l go --go-module-prefix github.com/secure-systems-lab
	docker run -v ${PWD}:/defs ${PROTOC_IMAGE} -i protos -f envelope.proto -l go --go-module-prefix github.com/secure-systems-lab/go-securesystemslib

[asraa]

This I was actually unsure about: the generated DSSE code lives here in protobuf-specs for now, and without that the imports for the other files that depend on it might get messed up. I actually think I might be able ot preserve the secure-systems-lab part, let me see.

[asraa]

Yeah :| I'm not sure I can preserve the module path outside of protobuf-specs... if I do, I end up with this error:

--go_out: github.com/sigstore/protobuf-specs/gen/pb-go/dsse/envelope.pb.go: generated file does not match prefix "github.com/secure-systems-lab/go-securesystemslib/gen/pb-go"

[znewman01]

Hold for more advice from somebody who understands who proto and Go interact but LGTM otherwise :)

[asraa]

@bobcallaway or @kommendorkapten can you give a quick review please?

[codysoyland]

Assuming we want this running on all pull requests, prior to merge, I think this needs to not have a branches key.

Suggested change

	pull_request:
	branches: [ main ]
	pull_request:
	types:
	- opened
	paths:
	- '**.proto'

Edit: updated with types and paths

[codysoyland]

I'm not an Actions expert, so you might need to play with these settings a bit until it starts building this PR.

[asraa]

Ohhh i think i can leave it blank maybe, with the paths

[asraa]

I think the branches is for targetted against, so understood if it's meant to capture all branches.

Either new workflows aren't triggered in the pull request itself or I've done something wrong still

[codysoyland]

Yeah, maybe it needs to be merged before it starts working. We can fix in a follow-up if not!

[kommendorkapten]

🚀

[asraa]

Just pinging for an approval: Updated with a Dockerfile for auto-updating protoc-all

[kommendorkapten]

🚀