From 6958ee8ff22628655cd9696c231b5dcf0a972b9c Mon Sep 17 00:00:00 2001 From: Benjamin Wang Date: Fri, 24 Jun 2022 09:38:52 +0800 Subject: [PATCH] Skip WatchRequestProgress test in grpc-proxy mode. We shouldn't fail the grpc-server (completely) by a not implemented RPC. Failing whole server by remote request is anti-pattern and security risk. Refer to https://github.com/etcd-io/etcd/runs/7034342964?check_suite_focus=true#step:5:2284 ``` === RUN TestWatchRequestProgress/1-watcher panic: not implemented goroutine 83024 [running]: go.etcd.io/etcd/proxy/grpcproxy.(*watchProxyStream).recvLoop(0xc009232f00, 0x4a73e1, 0xc00e2406e0) /home/runner/work/etcd/etcd/proxy/grpcproxy/watch.go:265 +0xbf2 go.etcd.io/etcd/proxy/grpcproxy.(*watchProxy).Watch.func1(0xc0038a3bc0, 0xc009232f00) /home/runner/work/etcd/etcd/proxy/grpcproxy/watch.go:125 +0x70 created by go.etcd.io/etcd/proxy/grpcproxy.(*watchProxy).Watch /home/runner/work/etcd/etcd/proxy/grpcproxy/watch.go:123 +0x73b FAIL go.etcd.io/etcd/clientv3/integration 222.813s FAIL ``` Signed-off-by: Benjamin Wang --- clientv3/integration/watch_test.go | 3 +++ integration/cluster_direct.go | 2 +- integration/cluster_proxy.go | 2 +- integration/v3_watch_test.go | 2 +- proxy/grpcproxy/watch.go | 3 ++- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/clientv3/integration/watch_test.go b/clientv3/integration/watch_test.go index b4086b8d10e..f553385a999 100644 --- a/clientv3/integration/watch_test.go +++ b/clientv3/integration/watch_test.go @@ -607,6 +607,9 @@ func TestConfigurableWatchProgressNotifyInterval(t *testing.T) { } func TestWatchRequestProgress(t *testing.T) { + if integration.ThroughProxy { + t.Skip("grpc-proxy does not support WatchProgress yet") + } testCases := []struct { name string watchers []string diff --git a/integration/cluster_direct.go b/integration/cluster_direct.go index 2479940c726..c64de0a2b73 100644 --- a/integration/cluster_direct.go +++ b/integration/cluster_direct.go @@ -23,7 +23,7 @@ import ( pb "go.etcd.io/etcd/etcdserver/etcdserverpb" ) -const throughProxy = false +const ThroughProxy = false func toGRPC(c *clientv3.Client) grpcAPI { return grpcAPI{ diff --git a/integration/cluster_proxy.go b/integration/cluster_proxy.go index 080b55f9072..020a58f5fbc 100644 --- a/integration/cluster_proxy.go +++ b/integration/cluster_proxy.go @@ -26,7 +26,7 @@ import ( "go.etcd.io/etcd/proxy/grpcproxy/adapter" ) -const throughProxy = true +const ThroughProxy = true var ( pmu sync.Mutex diff --git a/integration/v3_watch_test.go b/integration/v3_watch_test.go index ccc3f9a5614..ec2139a9a6c 100644 --- a/integration/v3_watch_test.go +++ b/integration/v3_watch_test.go @@ -1242,7 +1242,7 @@ func TestV3WatchCancellation(t *testing.T) { } var expected string - if throughProxy { + if ThroughProxy { // grpc proxy has additional 2 watches open expected = "3" } else { diff --git a/proxy/grpcproxy/watch.go b/proxy/grpcproxy/watch.go index 770941b8de9..4493319109e 100644 --- a/proxy/grpcproxy/watch.go +++ b/proxy/grpcproxy/watch.go @@ -262,7 +262,8 @@ func (wps *watchProxyStream) recvLoop() error { case *pb.WatchRequest_CancelRequest: wps.delete(uv.CancelRequest.WatchId) default: - panic("not implemented") + // Panic or Fatalf would allow network clients to crash the serve remotely. + //panic("not implemented") } } }