diff --git a/scripts/genproto.sh b/scripts/genproto.sh index 53e257608cc..d77b0fc1c8a 100755 --- a/scripts/genproto.sh +++ b/scripts/genproto.sh @@ -45,11 +45,21 @@ fi source ./scripts/test_lib.sh -if [[ $(protoc --version | cut -f2 -d' ') != "3.20.3" ]]; then - echo "Could not find protoc 3.20.3, installing now..." +GOOGLEAPI_ROOT=$(mktemp -d -t 'googleapi.XXXXX') +readonly googleapi_commit=0adf469dcd7822bf5bc058a7b0217f5558a75643 - arch=$(go env GOARCH) +readonly PROTOC_VERSION="3.20.3" +PROTOC_ROOT=$(mktemp -d -t 'protoc.XXXXX') + +function cleanup_temps() { + echo "Cleaning up ${PROTOC_ROOT} and ${GOOGLEAPI_ROOT}" + rm -rf "${PROTOC_ROOT}" + rm -rf "${GOOGLEAPI_ROOT}" +} +trap cleanup_temps EXIT +function download_protoc() { + arch=$(go env GOARCH) case ${arch} in "amd64") file="x86_64" ;; "arm64") file="aarch_64" ;; @@ -59,38 +69,40 @@ if [[ $(protoc --version | cut -f2 -d' ') != "3.20.3" ]]; then ;; esac - protoc_download_file="protoc-3.20.3-linux-${file}.zip" + protoc_download_file="protoc-${PROTOC_VERSION}-linux-${file}.zip" if [ "$OS" == "darwin" ]; then # protoc-3.20.3 does not have pre-built binaries for darwin_arm64. Thanks to Rosetta, we could use x86_64 binary. - protoc_download_file="protoc-3.20.3-osx-x86_64.zip" + protoc_download_file="protoc-${PROTOC_VERSION}-osx-x86_64.zip" fi - download_url="https://github.com/protocolbuffers/protobuf/releases/download/v3.20.3/${protoc_download_file}" + + download_url="https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/${protoc_download_file}" echo "Running on ${OS} ${arch}. Downloading ${protoc_download_file}" mkdir -p bin - wget ${download_url} && unzip -p ${protoc_download_file} bin/protoc > tmpFile && mv tmpFile bin/protoc + wget ${download_url} && unzip ${protoc_download_file} -d "${PROTOC_ROOT}" rm ${protoc_download_file} - chmod +x bin/protoc - PATH=$PATH:$(pwd)/bin - export PATH - echo "Now running: $(protoc --version)" -fi + chmod +x "${PROTOC_ROOT}/bin/protoc" + export PATH=${PROTOC_ROOT}/bin:${PATH} +} + +function check_protoc_installed() { + local version + + version=$(protoc --version | awk '{print $2}') + echo "protoc - $version" + if [ "${version}" != "${PROTOC_VERSION}" ]; then + echo "protoc version ${version} found, but v${PROTOC_VERSION} is required" + exit 1 + fi +} GOFAST_BIN=$(tool_get_bin github.com/gogo/protobuf/protoc-gen-gofast) +GO_PROTOGEN_BIN=$(tool_get_bin google.golang.org/protobuf/cmd/protoc-gen-go) GRPC_GATEWAY_BIN=$(tool_get_bin github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway) OPENAPIV2_BIN=$(tool_get_bin github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2) GOGOPROTO_ROOT="$(tool_pkg_dir github.com/gogo/protobuf/proto)/.." GRPC_GATEWAY_ROOT="$(tool_pkg_dir github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway)/.." RAFT_ROOT="$(tool_pkg_dir go.etcd.io/raft/v3/raftpb)/.." -GOOGLEAPI_ROOT=$(mktemp -d -t 'googleapi.XXXXX') - -readonly googleapi_commit=0adf469dcd7822bf5bc058a7b0217f5558a75643 - -function cleanup_googleapi() { - rm -rf "${GOOGLEAPI_ROOT}" -} - -trap cleanup_googleapi EXIT # TODO(ahrtr): use buf (https://github.com/bufbuild/buf) to manage the protobuf dependencies? function download_googleapi() { @@ -102,12 +114,16 @@ function download_googleapi() { run popd } +download_protoc +check_protoc_installed download_googleapi echo echo "Resolved binary and packages versions:" echo " - protoc-gen-gofast: ${GOFAST_BIN}" +echo " - protoc-gen-go: ${GO_PROTOGEN_BIN}" echo " - protoc-gen-grpc-gateway: ${GRPC_GATEWAY_BIN}" +echo " - protoc-root: ${PROTOC_ROOT}" echo " - openapiv2: ${OPENAPIV2_BIN}" echo " - gogoproto-root: ${GOGOPROTO_ROOT}" echo " - grpc-gateway-root: ${GRPC_GATEWAY_ROOT}" @@ -131,6 +147,17 @@ for dir in ${DIRS}; do run popd done +run pushd "./server/etcdserver/api/internalpb" + # Using non-gogo Go proto to keep compatibility with google.rpc.Status for grpc-gateway. + run protoc \ + --go_out=. \ + --go_opt=paths=source_relative \ + -I=".:${PROTOC_ROOT}/include" \ + ./*.proto + run gofmt -s -w ./*.pb.go + run_go_tool "golang.org/x/tools/cmd/goimports" -w ./*.pb.go +run popd + log_callout -e "\\nRunning swagger & grpc_gateway proto generation..." # remove old swagger files so it's obvious whether the files fail to generate diff --git a/server/embed/serve.go b/server/embed/serve.go index e50529dfe43..fdcd0245b7d 100644 --- a/server/embed/serve.go +++ b/server/embed/serve.go @@ -31,8 +31,10 @@ import ( "go.uber.org/zap" "golang.org/x/net/http2" "golang.org/x/net/trace" + statuspb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc" "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" etcdservergw "go.etcd.io/etcd/api/v3/etcdserverpb/gw" "go.etcd.io/etcd/client/pkg/v3/transport" @@ -40,6 +42,7 @@ import ( "go.etcd.io/etcd/pkg/v3/httputil" "go.etcd.io/etcd/server/v3/config" "go.etcd.io/etcd/server/v3/etcdserver" + "go.etcd.io/etcd/server/v3/etcdserver/api/internalpb" "go.etcd.io/etcd/server/v3/etcdserver/api/v3client" "go.etcd.io/etcd/server/v3/etcdserver/api/v3election" "go.etcd.io/etcd/server/v3/etcdserver/api/v3election/v3electionpb" @@ -342,6 +345,18 @@ func (sctx *serveCtx) registerGateway(dial func(ctx context.Context) (*grpc.Clie }, }, ), + gw.WithForwardResponseRewriter(func(_ context.Context, resp proto.Message) (any, error) { + st, ok := resp.(*statuspb.Status) + if !ok || st == nil { + return resp, nil + } + return &internalpb.GRPCGatewayError{ + Error: st.GetMessage(), + Code: st.GetCode(), + Message: st.GetMessage(), + Details: st.GetDetails(), + }, nil + }), ) handlers := []registerHandlerFunc{ diff --git a/server/etcdserver/api/internalpb/error.pb.go b/server/etcdserver/api/internalpb/error.pb.go new file mode 100644 index 00000000000..d1b2864429c --- /dev/null +++ b/server/etcdserver/api/internalpb/error.pb.go @@ -0,0 +1,161 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.11 +// protoc v3.20.3 +// source: error.proto + +package internalpb + +import ( + reflect "reflect" + sync "sync" + unsafe "unsafe" + + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + anypb "google.golang.org/protobuf/types/known/anypb" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +// GRPCGatewayError preserves the legacy "error" field in grpc-gateway JSON +// responses for backward compatibility with existing clients and tooling. +// +// The "error" field is deprecated in v3.7 and will be removed in v3.8; use +// the standard gRPC Status fields ("code", "message", "details") instead. +// +// REF: https://github.com/grpc-ecosystem/grpc-gateway/issues/1098 +type GRPCGatewayError struct { + state protoimpl.MessageState `protogen:"open.v1"` + Error string `protobuf:"bytes,1,opt,name=error,proto3" json:"error,omitempty"` + Code int32 `protobuf:"varint,2,opt,name=code,proto3" json:"code,omitempty"` + Message string `protobuf:"bytes,3,opt,name=message,proto3" json:"message,omitempty"` + Details []*anypb.Any `protobuf:"bytes,4,rep,name=details,proto3" json:"details,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *GRPCGatewayError) Reset() { + *x = GRPCGatewayError{} + mi := &file_error_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *GRPCGatewayError) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*GRPCGatewayError) ProtoMessage() {} + +func (x *GRPCGatewayError) ProtoReflect() protoreflect.Message { + mi := &file_error_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use GRPCGatewayError.ProtoReflect.Descriptor instead. +func (*GRPCGatewayError) Descriptor() ([]byte, []int) { + return file_error_proto_rawDescGZIP(), []int{0} +} + +func (x *GRPCGatewayError) GetError() string { + if x != nil { + return x.Error + } + return "" +} + +func (x *GRPCGatewayError) GetCode() int32 { + if x != nil { + return x.Code + } + return 0 +} + +func (x *GRPCGatewayError) GetMessage() string { + if x != nil { + return x.Message + } + return "" +} + +func (x *GRPCGatewayError) GetDetails() []*anypb.Any { + if x != nil { + return x.Details + } + return nil +} + +var File_error_proto protoreflect.FileDescriptor + +const file_error_proto_rawDesc = "" + + "\n" + + "\verror.proto\x12\n" + + "internalpb\x1a\x19google/protobuf/any.proto\"\x86\x01\n" + + "\x10GRPCGatewayError\x12\x14\n" + + "\x05error\x18\x01 \x01(\tR\x05error\x12\x12\n" + + "\x04code\x18\x02 \x01(\x05R\x04code\x12\x18\n" + + "\amessage\x18\x03 \x01(\tR\amessage\x12.\n" + + "\adetails\x18\x04 \x03(\v2\x14.google.protobuf.AnyR\adetailsB5Z3go.etcd.io/etcd/server/v3/etcdserver/api/internalpbb\x06proto3" + +var ( + file_error_proto_rawDescOnce sync.Once + file_error_proto_rawDescData []byte +) + +func file_error_proto_rawDescGZIP() []byte { + file_error_proto_rawDescOnce.Do(func() { + file_error_proto_rawDescData = protoimpl.X.CompressGZIP(unsafe.Slice(unsafe.StringData(file_error_proto_rawDesc), len(file_error_proto_rawDesc))) + }) + return file_error_proto_rawDescData +} + +var file_error_proto_msgTypes = make([]protoimpl.MessageInfo, 1) +var file_error_proto_goTypes = []any{ + (*GRPCGatewayError)(nil), // 0: internalpb.GRPCGatewayError + (*anypb.Any)(nil), // 1: google.protobuf.Any +} +var file_error_proto_depIdxs = []int32{ + 1, // 0: internalpb.GRPCGatewayError.details:type_name -> google.protobuf.Any + 1, // [1:1] is the sub-list for method output_type + 1, // [1:1] is the sub-list for method input_type + 1, // [1:1] is the sub-list for extension type_name + 1, // [1:1] is the sub-list for extension extendee + 0, // [0:1] is the sub-list for field type_name +} + +func init() { file_error_proto_init() } +func file_error_proto_init() { + if File_error_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: unsafe.Slice(unsafe.StringData(file_error_proto_rawDesc), len(file_error_proto_rawDesc)), + NumEnums: 0, + NumMessages: 1, + NumExtensions: 0, + NumServices: 0, + }, + GoTypes: file_error_proto_goTypes, + DependencyIndexes: file_error_proto_depIdxs, + MessageInfos: file_error_proto_msgTypes, + }.Build() + File_error_proto = out.File + file_error_proto_goTypes = nil + file_error_proto_depIdxs = nil +} diff --git a/server/etcdserver/api/internalpb/error.proto b/server/etcdserver/api/internalpb/error.proto new file mode 100644 index 00000000000..21e5566dde1 --- /dev/null +++ b/server/etcdserver/api/internalpb/error.proto @@ -0,0 +1,20 @@ +syntax = "proto3"; +package internalpb; + +import "google/protobuf/any.proto"; + +option go_package = "go.etcd.io/etcd/server/v3/etcdserver/api/internalpb"; + +// GRPCGatewayError preserves the legacy "error" field in grpc-gateway JSON +// responses for backward compatibility with existing clients and tooling. +// +// The "error" field is deprecated in v3.7 and will be removed in v3.8; use +// the standard gRPC Status fields ("code", "message", "details") instead. +// +// REF: https://github.com/grpc-ecosystem/grpc-gateway/issues/1098 +message GRPCGatewayError { + string error = 1; + int32 code = 2; + string message = 3; + repeated google.protobuf.Any details = 4; +} diff --git a/server/go.mod b/server/go.mod index 454890ec3a7..b50081d9554 100644 --- a/server/go.mod +++ b/server/go.mod @@ -41,6 +41,7 @@ require ( golang.org/x/net v0.49.0 golang.org/x/time v0.14.0 google.golang.org/genproto/googleapis/api v0.0.0-20251222181119-0a764e51fe1b + google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b google.golang.org/grpc v1.78.0 google.golang.org/protobuf v1.36.11 gopkg.in/natefinch/lumberjack.v2 v2.2.1 @@ -72,7 +73,6 @@ require ( go.yaml.in/yaml/v2 v2.4.3 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.33.0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/tests/e2e/v3_curl_auth_test.go b/tests/e2e/v3_curl_auth_test.go index c8745d23179..d3a49842491 100644 --- a/tests/e2e/v3_curl_auth_test.go +++ b/tests/e2e/v3_curl_auth_test.go @@ -58,6 +58,10 @@ func TestCurlV3AuthEnableDisableStatus(t *testing.T) { testCtl(t, testCurlV3AuthEnableDisableStatus) } +func TestCurlV3AuthNotEnabledErrorField(t *testing.T) { + testCtl(t, testCurlV3AuthNotEnabledErrorField) +} + func testCurlV3Auth(cx ctlCtx) { usernames := []string{"root", "nonroot", "nooption"} pwds := []string{"toor", "pass", "pass"} @@ -408,3 +412,28 @@ func testCurlV3AuthEnableDisableStatus(cx ctlCtx) { Expected: expect.ExpectedResponse{Value: "revision"}, }), "testCurlV3AuthEnableDisableStatus failed to get auth status") } + +func testCurlV3AuthNotEnabledErrorField(cx ctlCtx) { + authreq, err := json.Marshal(&pb.AuthenticateRequest{Name: "root", Password: "toor"}) + require.NoError(cx.t, err) + + args := e2e.CURLPrefixArgsCluster(cx.epc.Cfg, cx.epc.Procs[0], "POST", e2e.CURLReq{ + Endpoint: "/v3/auth/authenticate", + Value: string(authreq), + }) + resp, err := runCommandAndReadJSONOutput(args) + require.NoError(cx.t, err) + + errorField, ok := resp["error"].(string) + require.Truef(cx.t, ok, "grpc-gateway error field missing or not a string: %v", resp) + + messageField, ok := resp["message"].(string) + require.Truef(cx.t, ok, "grpc-gateway message field missing or not a string: %v", resp) + + require.Equalf(cx.t, messageField, errorField, "grpc-gateway error field should match message") + require.Equal(cx.t, "etcdserver: authentication is not enabled", messageField) + + codeField, ok := resp["code"].(float64) + require.Truef(cx.t, ok, "grpc-gateway code field missing or not a number: %v", resp) + require.Equal(cx.t, 9, int(codeField)) +} diff --git a/tools/mod/go.mod b/tools/mod/go.mod index b724e479beb..b63e28dbb1e 100644 --- a/tools/mod/go.mod +++ b/tools/mod/go.mod @@ -17,6 +17,7 @@ require ( go.etcd.io/protodoc v0.0.0-20180829002748-484ab544e116 go.etcd.io/raft/v3 v3.6.0 golang.org/x/tools v0.41.0 + google.golang.org/protobuf v1.36.11 gotest.tools/gotestsum v1.13.0 gotest.tools/v3 v3.5.2 honnef.co/go/tools v0.6.1 @@ -249,7 +250,6 @@ require ( google.golang.org/genproto/googleapis/api v0.0.0-20251222181119-0a764e51fe1b // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b // indirect google.golang.org/grpc v1.78.0 // indirect - google.golang.org/protobuf v1.36.11 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect mvdan.cc/gofumpt v0.9.2 // indirect diff --git a/tools/mod/tools.go b/tools/mod/tools.go index e88aa4a7a1c..ea16ddf2757 100644 --- a/tools/mod/tools.go +++ b/tools/mod/tools.go @@ -31,6 +31,7 @@ import ( _ "github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2" _ "github.com/ryancurrah/gomodguard/cmd/gomodguard" _ "golang.org/x/tools/cmd/goimports" + _ "google.golang.org/protobuf/cmd/protoc-gen-go" _ "gotest.tools/gotestsum" _ "gotest.tools/v3" _ "honnef.co/go/tools/cmd/staticcheck"