From 5f9a1394db444bc8cebbcbc19b3bc3221894c87c Mon Sep 17 00:00:00 2001 From: Piotr Tabor Date: Thu, 17 Sep 2020 08:58:22 +0200 Subject: [PATCH] integration,proxy: Skip WatchRequestProgress test in grpc-proxy mode. Fixes: go test -tags cluster_proxy ./clientv3/integration -v -run TestWatchRequestProgress Does not fail the grpc-server (completely) by a not implemented RPC. Failing whole server by remote request is anti-pattern and security risk. Prior to the fix, the command line above was failing with: ``` === RUN TestWatchRequestProgress/0-watcher panic: not implemented goroutine 602 [running]: go.etcd.io/etcd/v3/proxy/grpcproxy.(*watchProxyStream).recvLoop(0xc0004779d0, 0x0, 0x0) /home/ptab/corp/etcd/proxy/grpcproxy/watch.go:275 +0xac5 go.etcd.io/etcd/v3/proxy/grpcproxy.(*watchProxy).Watch.func1(0xc0034f94a0, 0xc0004779d0) /home/ptab/corp/etcd/proxy/grpcproxy/watch.go:129 +0x53 created by go.etcd.io/etcd/v3/proxy/grpcproxy.(*watchProxy).Watch /home/ptab/corp/etcd/proxy/grpcproxy/watch.go:127 +0x3c8 FAIL go.etcd.io/etcd/v3/clientv3/integration 0.215s FAIL ``` --- clientv3/integration/watch_test.go | 3 +++ integration/cluster_direct.go | 2 +- integration/cluster_proxy.go | 2 +- integration/v3_watch_test.go | 4 ++-- proxy/grpcproxy/watch.go | 3 ++- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/clientv3/integration/watch_test.go b/clientv3/integration/watch_test.go index ce5d7ed0e90..e33e7605a1c 100644 --- a/clientv3/integration/watch_test.go +++ b/clientv3/integration/watch_test.go @@ -609,6 +609,9 @@ func TestConfigurableWatchProgressNotifyInterval(t *testing.T) { } func TestWatchRequestProgress(t *testing.T) { + if integration.ThroughProxy { + t.Skipf("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 a3764d9ced3..0db7b232ee3 100644 --- a/integration/cluster_direct.go +++ b/integration/cluster_direct.go @@ -23,7 +23,7 @@ import ( pb "go.etcd.io/etcd/v3/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 0b024c71995..b5e2b957113 100644 --- a/integration/cluster_proxy.go +++ b/integration/cluster_proxy.go @@ -27,7 +27,7 @@ import ( "go.uber.org/zap" ) -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 1717c05371d..38cc17dd9af 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 { @@ -1279,7 +1279,7 @@ func TestV3WatchCloseCancelRace(t *testing.T) { } var expected string - if throughProxy { + if ThroughProxy { // grpc proxy has additional 2 watches open expected = "2" } else { diff --git a/proxy/grpcproxy/watch.go b/proxy/grpcproxy/watch.go index fc62c4aa399..bbde0ad591e 100644 --- a/proxy/grpcproxy/watch.go +++ b/proxy/grpcproxy/watch.go @@ -272,7 +272,8 @@ func (wps *watchProxyStream) recvLoop() error { wps.delete(uv.CancelRequest.WatchId) wps.lg.Debug("cancel watcher", zap.Int64("watcherId", uv.CancelRequest.WatchId)) default: - panic("not implemented") + // Panic or Fatalf would allow to network clients to crash the serve remotely. + wps.lg.Error("not supported request type by gRPC proxy", zap.Stringer("request", req)) } } }