From 523e054c81368634f95452589accb357b7f09cc4 Mon Sep 17 00:00:00 2001 From: Eric Date: Fri, 25 Mar 2022 09:30:30 -0400 Subject: [PATCH] assorted changes required to remove gogo --- .circleci/config.yml | 13 +++++---- agent/http.go | 6 ++-- agent/structs/protobuf_compat.go | 17 ----------- agent/structs/structs.go | 32 +++++++++++++++++++-- build-support/scripts/proto-gen-no-gogo.sh | 33 ++++++++++++++++------ proto/pbcommon/common.gen.go | 12 ++++++++ proto/pbcommon/common.go | 29 ------------------- proto/pbcommon/common.pb.go | 6 ++++ proto/pbcommon/common.proto | 6 ++++ 9 files changed, 90 insertions(+), 64 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 64395e67b..e637d0d08 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -232,6 +232,7 @@ jobs: wget https://github.com/protocolbuffers/protobuf/releases/download/v3.12.3/protoc-3.12.3-linux-x86_64.zip sudo unzip -d /usr/local protoc-*.zip sudo chmod +x /usr/local/bin/protoc + sudo chmod -R a+Xr /usr/local/include/google/ rm protoc-*.zip - run: name: Install gogo/protobuf @@ -239,6 +240,8 @@ jobs: gogo_version=$(go list -m github.com/gogo/protobuf | awk '{print $2}') go install -v github.com/hashicorp/protoc-gen-go-binary@master go install -v github.com/gogo/protobuf/protoc-gen-gofast@${gogo_version} + go install -v github.com/favadi/protoc-go-inject-tag@v1.3.0 + go install -v github.com/golang/protobuf/protoc-gen-go@v1.3.5 - run: command: make --always-make proto @@ -278,7 +281,7 @@ jobs: fi - run-go-test-full: go_test_flags: 'if ! [[ "$CIRCLE_BRANCH" =~ ^main$|^release/ ]]; then export GO_TEST_FLAGS="-short"; fi' - + go-test: docker: - image: *GOLANG_IMAGE @@ -330,9 +333,9 @@ jobs: path: /tmp/jsonfile - run: *notify-slack-failure - # go-test-32bit is to catch problems where 64-bit ints must be 64-bit aligned + # go-test-32bit is to catch problems where 64-bit ints must be 64-bit aligned # to use them with sync/atomic. See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. - # Running tests with GOARCH=386 seems to be the best way to detect this + # Running tests with GOARCH=386 seems to be the best way to detect this # problem. Only runs tests that are -short to limit the time we spend checking # for these bugs. go-test-32bit: @@ -747,11 +750,11 @@ jobs: if ! git diff --quiet --exit-code HEAD^! ui/; then git config --local user.email "github-team-consul-core@hashicorp.com" git config --local user.name "hc-github-team-consul-core" - + # -B resets the CI branch to main which may diverge history # but we will force push anyways. git checkout -B ci/main-assetfs-build main - + short_sha=$(git rev-parse --short HEAD) git add agent/uiserver/bindata_assetfs.go git commit -m "auto-updated agent/uiserver/bindata_assetfs.go from commit ${short_sha}" diff --git a/agent/http.go b/agent/http.go index 90885bc3f..e039c2c7c 100644 --- a/agent/http.go +++ b/agent/http.go @@ -31,7 +31,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/logging" - "github.com/hashicorp/consul/proto/pbcommongogo" + "github.com/hashicorp/consul/proto/pbcommon" ) var HTTPSummaries = []prometheus.SummaryDefinition{ @@ -781,7 +781,7 @@ func setLastContact(resp http.ResponseWriter, last time.Duration) { } // setMeta is used to set the query response meta data -func setMeta(resp http.ResponseWriter, m structs.QueryMetaCompat) error { +func setMeta(resp http.ResponseWriter, m *structs.QueryMeta) error { lastContact, err := m.GetLastContact() if err != nil { return err @@ -981,7 +981,7 @@ func (s *HTTPHandlers) parseConsistency(resp http.ResponseWriter, req *http.Requ } // parseConsistencyReadRequest is used to parse the ?consistent query param. -func parseConsistencyReadRequest(resp http.ResponseWriter, req *http.Request, b *pbcommongogo.ReadRequest) { +func parseConsistencyReadRequest(resp http.ResponseWriter, req *http.Request, b *pbcommon.ReadRequest) { query := req.URL.Query() if _, ok := query["consistent"]; ok { b.RequireConsistent = true diff --git a/agent/structs/protobuf_compat.go b/agent/structs/protobuf_compat.go index 93358e8e3..143bd97e3 100644 --- a/agent/structs/protobuf_compat.go +++ b/agent/structs/protobuf_compat.go @@ -32,23 +32,6 @@ type QueryOptionsCompat interface { SetFilter(string) } -// QueryMetaCompat is the interface that both the structs.QueryMeta -// and the proto/pbcommongogo.QueryMeta structs need to implement so that they -// can be operated on interchangeably -type QueryMetaCompat interface { - GetLastContact() (time.Duration, error) - SetLastContact(time.Duration) - GetKnownLeader() bool - SetKnownLeader(bool) - GetIndex() uint64 - SetIndex(uint64) - GetConsistencyLevel() string - SetConsistencyLevel(string) - GetBackend() QueryBackend - GetResultsFilteredByACLs() bool - SetResultsFilteredByACLs(bool) -} - // GetToken helps implement the QueryOptionsCompat interface // Copied from proto/pbcommongogo/common.pb.go func (m *QueryOptions) GetToken() string { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 9efd02c9b..ca4a7c849 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -16,6 +16,7 @@ import ( "strings" "time" + "github.com/golang/protobuf/proto" "github.com/hashicorp/consul-net-rpc/go-msgpack/codec" "github.com/hashicorp/go-multierror" "github.com/hashicorp/serf/coordinate" @@ -2575,13 +2576,17 @@ type ProtoMarshaller interface { func EncodeProtoInterface(t MessageType, message interface{}) ([]byte, error) { if marshaller, ok := message.(ProtoMarshaller); ok { + return EncodeProtoGogo(t, marshaller) + } + + if marshaller, ok := message.(proto.Message); ok { return EncodeProto(t, marshaller) } return nil, fmt.Errorf("message does not implement the ProtoMarshaller interface: %T", message) } -func EncodeProto(t MessageType, message ProtoMarshaller) ([]byte, error) { +func EncodeProtoGogo(t MessageType, message ProtoMarshaller) ([]byte, error) { data := make([]byte, message.Size()+1) data[0] = uint8(t) if _, err := message.MarshalTo(data[1:]); err != nil { @@ -2590,7 +2595,24 @@ func EncodeProto(t MessageType, message ProtoMarshaller) ([]byte, error) { return data, nil } -func DecodeProto(buf []byte, out ProtoMarshaller) error { +func EncodeProto(t MessageType, pb proto.Message) ([]byte, error) { + data := make([]byte, proto.Size(pb)+1) + data[0] = uint8(t) + + buf := proto.NewBuffer(data[1:1]) + if err := buf.Marshal(pb); err != nil { + return nil, err + } + + return data, nil +} + +func DecodeProto(buf []byte, pb proto.Message) error { + // Note that this assumes the leading byte indicating the type as already been stripped off. + return proto.Unmarshal(buf, pb) +} + +func DecodeProtoGogo(buf []byte, out ProtoMarshaller) error { // Note that this assumes the leading byte indicating the type as already been stripped off. return out.Unmarshal(buf) } @@ -2720,3 +2742,9 @@ func TimeToProto(s time.Time) *timestamp.Timestamp { ret, _ := ptypes.TimestampProto(s) return ret } + +// IsZeroProtoTime returns true if the time is the minimum protobuf timestamp +// (the Unix epoch). +func IsZeroProtoTime(t *timestamp.Timestamp) bool { + return t.Seconds == 0 && t.Nanos == 0 +} diff --git a/build-support/scripts/proto-gen-no-gogo.sh b/build-support/scripts/proto-gen-no-gogo.sh index c736d49de..0585a02d7 100755 --- a/build-support/scripts/proto-gen-no-gogo.sh +++ b/build-support/scripts/proto-gen-no-gogo.sh @@ -68,6 +68,8 @@ function main { return 1 fi + go mod download + local golang_proto_path=$(go list -f '{{ .Dir }}' -m github.com/golang/protobuf) local golang_proto_mod_path=$(sed -e 's,\(.*\)github.com.*,\1,' <<< "${golang_proto_path}") @@ -77,7 +79,7 @@ function main { local proto_go_path=${proto_path%%.proto}.pb.go local proto_go_bin_path=${proto_path%%.proto}.pb.binary.go - + local go_proto_out="paths=source_relative" if is_set "${grpc}" then @@ -96,10 +98,17 @@ function main { # How we run protoc probably needs some documentation. # - # This is the path to where + # This is the path to where # -I="${golang_proto_path}/protobuf" \ local -i ret=0 status_stage "Generating ${proto_path} into ${proto_go_path} and ${proto_go_bin_path} (NO GOGO)" + echo "debug_run protoc \ + -I=\"${golang_proto_path}\" \ + -I=\"${golang_proto_mod_path}\" \ + -I=\"${SOURCE_DIR}\" \ + --go_out=\"${go_proto_out}${SOURCE_DIR}\" \ + --go-binary_out=\"${SOURCE_DIR}\" \ + \"${proto_path}\"" debug_run protoc \ -I="${golang_proto_path}" \ -I="${golang_proto_mod_path}" \ @@ -107,9 +116,22 @@ function main { --go_out="${go_proto_out}${SOURCE_DIR}" \ --go-binary_out="${SOURCE_DIR}" \ "${proto_path}" + + if test $? -ne 0 + then + err "Failed to run protoc for ${proto_path}" + return 1 + fi + debug_run protoc-go-inject-tag \ -input="${proto_go_path}" + if test $? -ne 0 + then + err "Failed to run protoc-go-inject-tag for ${proto_path}" + return 1 + fi + echo "debug_run protoc \ -I=\"${golang_proto_path}\" \ -I=\"${golang_proto_mod_path}\" \ @@ -117,11 +139,6 @@ function main { --go_out=\"${go_proto_out}${SOURCE_DIR}\" \ --go-binary_out=\"${SOURCE_DIR}\" \ \"${proto_path}\"" - if test $? -ne 0 - then - err "Failed to generate outputs from ${proto_path}" - return 1 - fi BUILD_TAGS=$(sed -e '/^[[:space:]]*$/,$d' < "${proto_path}" | grep '// +build') if test -n "${BUILD_TAGS}" @@ -129,7 +146,7 @@ function main { echo -e "${BUILD_TAGS}\n" >> "${proto_go_path}.new" cat "${proto_go_path}" >> "${proto_go_path}.new" mv "${proto_go_path}.new" "${proto_go_path}" - + echo -e "${BUILD_TAGS}\n" >> "${proto_go_bin_path}.new" cat "${proto_go_bin_path}" >> "${proto_go_bin_path}.new" mv "${proto_go_bin_path}.new" "${proto_go_bin_path}" diff --git a/proto/pbcommon/common.gen.go b/proto/pbcommon/common.gen.go index 867e8089c..636931f81 100644 --- a/proto/pbcommon/common.gen.go +++ b/proto/pbcommon/common.gen.go @@ -68,3 +68,15 @@ func RaftIndexFromStructs(t *structs.RaftIndex, s *RaftIndex) { s.CreateIndex = t.CreateIndex s.ModifyIndex = t.ModifyIndex } +func WriteRequestToStructs(s *WriteRequest, t *structs.WriteRequest) { + if s == nil { + return + } + t.Token = s.Token +} +func WriteRequestFromStructs(t *structs.WriteRequest, s *WriteRequest) { + if s == nil { + return + } + s.Token = t.Token +} diff --git a/proto/pbcommon/common.go b/proto/pbcommon/common.go index fbff7e4ae..713089e48 100644 --- a/proto/pbcommon/common.go +++ b/proto/pbcommon/common.go @@ -88,35 +88,6 @@ func (q *QueryOptions) SetFilter(filter string) { q.Filter = filter } -// SetLastContact is needed to implement the structs.QueryMetaCompat interface -func (q *QueryMeta) SetLastContact(lastContact time.Duration) { - q.LastContact = structs.DurationToProto(lastContact) -} - -// SetKnownLeader is needed to implement the structs.QueryMetaCompat interface -func (q *QueryMeta) SetKnownLeader(knownLeader bool) { - q.KnownLeader = knownLeader -} - -// SetIndex is needed to implement the structs.QueryMetaCompat interface -func (q *QueryMeta) SetIndex(index uint64) { - q.Index = index -} - -// SetConsistencyLevel is needed to implement the structs.QueryMetaCompat interface -func (q *QueryMeta) SetConsistencyLevel(consistencyLevel string) { - q.ConsistencyLevel = consistencyLevel -} - -func (q *QueryMeta) GetBackend() structs.QueryBackend { - return structs.QueryBackend(0) -} - -// SetResultsFilteredByACLs is needed to implement the structs.QueryMetaCompat interface -func (q *QueryMeta) SetResultsFilteredByACLs(v bool) { - q.ResultsFilteredByACLs = v -} - // WriteRequest only applies to writes, always false // // IsRead implements structs.RPCInfo diff --git a/proto/pbcommon/common.pb.go b/proto/pbcommon/common.pb.go index 88a2d55b6..a04042cac 100644 --- a/proto/pbcommon/common.pb.go +++ b/proto/pbcommon/common.pb.go @@ -120,6 +120,12 @@ func (m *TargetDatacenter) GetDatacenter() string { return "" } +// mog annotation: +// +// target=github.com/hashicorp/consul/agent/structs.WriteRequest +// output=common.gen.go +// name=Structs +// ignore-fields=state,sizeCache,unknownFields type WriteRequest struct { // Token is the ACL token ID. If not provided, the 'anonymous' // token is assumed for backwards compatibility. diff --git a/proto/pbcommon/common.proto b/proto/pbcommon/common.proto index 19efd232b..e21b677f5 100644 --- a/proto/pbcommon/common.proto +++ b/proto/pbcommon/common.proto @@ -29,6 +29,12 @@ message TargetDatacenter { string Datacenter = 1; } +// mog annotation: +// +// target=github.com/hashicorp/consul/agent/structs.WriteRequest +// output=common.gen.go +// name=Structs +// ignore-fields=state,sizeCache,unknownFields message WriteRequest { // Token is the ACL token ID. If not provided, the 'anonymous' // token is assumed for backwards compatibility.