Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 48 additions & 21 deletions scripts/genproto.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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" ;;
Expand All @@ -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() {
Expand All @@ -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}"
Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions server/embed/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ 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"
"go.etcd.io/etcd/pkg/v3/debugutil"
"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"
Expand Down Expand Up @@ -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{
Expand Down
161 changes: 161 additions & 0 deletions server/etcdserver/api/internalpb/error.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions server/etcdserver/api/internalpb/error.proto
Original file line number Diff line number Diff line change
@@ -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.
Comment on lines +11 to +12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IF we plan to remove it in 3.8, then we don't need to resolve the issue at all.

The key for now is that we should formally document/standardize the error response format, either on grpc-gateway side or etcd side. Once finalized, we shouldn't change it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think we can just document this going forward, since the HTTP status code is correct and the error field is duplicated with message. If we all agree not to resolve it, that should be sufficient.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just document this going forward

If we don't change anything, it means we depend on whatever grpc-gateway provides. We should link to grpc-gateway's doc if any.

//
// 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;
}
2 changes: 1 addition & 1 deletion server/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)

Expand Down
29 changes: 29 additions & 0 deletions tests/e2e/v3_curl_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down Expand Up @@ -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))
}
Loading