From 136ef5d48b8d659d8ffcc1f546751315e9bd09b4 Mon Sep 17 00:00:00 2001 From: DerekBum Date: Tue, 5 Mar 2024 15:29:05 +0300 Subject: [PATCH] pool: get instance status from `WatchOnce` request Starting from Tarantool version >= 3.0.0 `WatchOnce` requset is supported. So we can get instance status using this request instead of calling `box.info`. This way user can add instances to the ConnectionPool without the `execute` access. Closes #380 --- CHANGELOG.md | 3 ++ pool/config.lua | 3 ++ pool/connection_pool.go | 17 +++++++- pool/connection_pool_test.go | 85 ++++++++++++++++++++++++++++++++++++ tarantool_test.go | 8 +--- test_helpers/utils.go | 8 ++++ 6 files changed, 115 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aecdf37ea..24f4daacf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. ### Changed +- `execute` access for `box.info` is no longer required for ConnectionPool + for a Tarantool version >= 3.0.0 (#380) + ### Fixed - `ConnectionPool.Remove()` does not notify a `ConnectionHandler` after diff --git a/pool/config.lua b/pool/config.lua index 4df392ba8..4e91a0d83 100644 --- a/pool/config.lua +++ b/pool/config.lua @@ -9,6 +9,9 @@ box.once("init", function() box.schema.user.create('test', { password = 'test' }) box.schema.user.grant('test', 'read,write,execute', 'universe') + box.schema.user.create('test_noexec', { password = 'test' }) + box.schema.user.grant('test_noexec', 'read,write', 'universe') + local s = box.schema.space.create('testPool', { id = 520, if_not_exists = true, diff --git a/pool/connection_pool.go b/pool/connection_pool.go index b4062c387..949b468bc 100644 --- a/pool/connection_pool.go +++ b/pool/connection_pool.go @@ -1017,7 +1017,20 @@ func (p *ConnectionPool) DoInstance(req tarantool.Request, name string) *taranto // func (p *ConnectionPool) getConnectionRole(conn *tarantool.Connection) (Role, error) { - data, err := conn.Do(tarantool.NewCallRequest("box.info")).Get() + var ( + roFieldName string + data []interface{} + err error + ) + + if isFeatureInSlice(iproto.IPROTO_FEATURE_WATCH_ONCE, conn.ProtocolInfo().Features) { + roFieldName = "is_ro" + data, err = conn.Do(tarantool.NewWatchOnceRequest("box.status")).Get() + } else { + roFieldName = "ro" + data, err = conn.Do(tarantool.NewCallRequest("box.info")).Get() + } + if err != nil { return UnknownRole, err } @@ -1033,7 +1046,7 @@ func (p *ConnectionPool) getConnectionRole(conn *tarantool.Connection) (Role, er return UnknownRole, ErrIncorrectStatus } - replicaRole, ok := data[0].(map[interface{}]interface{})["ro"] + replicaRole, ok := data[0].(map[interface{}]interface{})[roFieldName] if !ok { return UnknownRole, ErrIncorrectResponse } diff --git a/pool/connection_pool_test.go b/pool/connection_pool_test.go index 2592c7117..c4e43e2d3 100644 --- a/pool/connection_pool_test.go +++ b/pool/connection_pool_test.go @@ -1,6 +1,7 @@ package pool_test import ( + "bytes" "context" "fmt" "log" @@ -22,6 +23,7 @@ import ( ) var user = "test" +var userNoExec = "test_noexec" var pass = "test" var spaceNo = uint32(520) var spaceName = "testPool" @@ -68,6 +70,18 @@ func makeInstance(server string, opts tarantool.Opts) pool.Instance { } } +func makeNoExecuteInstance(server string, opts tarantool.Opts) pool.Instance { + return pool.Instance{ + Name: server, + Dialer: tarantool.NetDialer{ + Address: server, + User: userNoExec, + Password: pass, + }, + Opts: opts, + } +} + func makeInstances(servers []string, opts tarantool.Opts) []pool.Instance { var instances []pool.Instance for _, server := range servers { @@ -130,6 +144,77 @@ func TestConnSuccessfully(t *testing.T) { require.Nil(t, err) } +func TestConn_no_execute_supported(t *testing.T) { + test_helpers.SkipIfWatchOnceUnsupported(t) + + healthyServ := servers[0] + + ctx, cancel := test_helpers.GetPoolConnectContext() + defer cancel() + connPool, err := pool.Connect(ctx, + []pool.Instance{makeNoExecuteInstance(healthyServ, connOpts)}) + require.Nilf(t, err, "failed to connect") + require.NotNilf(t, connPool, "conn is nil after Connect") + + defer connPool.Close() + + args := test_helpers.CheckStatusesArgs{ + ConnPool: connPool, + Mode: pool.ANY, + Servers: []string{healthyServ}, + ExpectedPoolStatus: true, + ExpectedStatuses: map[string]bool{ + healthyServ: true, + }, + } + + err = test_helpers.CheckPoolStatuses(args) + require.Nil(t, err) + + _, err = connPool.Do(tarantool.NewPingRequest(), pool.ANY).Get() + require.Nil(t, err) +} + +func TestConn_no_execute_unsupported(t *testing.T) { + test_helpers.SkipIfWatchOnceSupported(t) + + var buf bytes.Buffer + log.SetOutput(&buf) + defer log.SetOutput(os.Stderr) + + healthyServ := servers[0] + + ctx, cancel := test_helpers.GetPoolConnectContext() + defer cancel() + connPool, err := pool.Connect(ctx, + []pool.Instance{makeNoExecuteInstance(healthyServ, connOpts)}) + require.Nilf(t, err, "failed to connect") + require.NotNilf(t, connPool, "conn is nil after Connect") + + defer connPool.Close() + + require.Contains(t, buf.String(), + fmt.Sprintf("connect to %s failed: Execute access to function "+ + "'box.info' is denied for user '%s'", servers[0], userNoExec)) + + args := test_helpers.CheckStatusesArgs{ + ConnPool: connPool, + Mode: pool.ANY, + Servers: []string{healthyServ}, + ExpectedPoolStatus: false, + ExpectedStatuses: map[string]bool{ + healthyServ: false, + }, + } + + err = test_helpers.CheckPoolStatuses(args) + require.Nil(t, err) + + _, err = connPool.Do(tarantool.NewPingRequest(), pool.ANY).Get() + require.Error(t, err) + require.Equal(t, "can't find healthy instance in pool", err.Error()) +} + func TestConnect_empty(t *testing.T) { cases := []struct { Name string diff --git a/tarantool_test.go b/tarantool_test.go index 0bd3f394b..03b786f33 100644 --- a/tarantool_test.go +++ b/tarantool_test.go @@ -2591,13 +2591,7 @@ func TestConnectionDoWatchOnceRequest(t *testing.T) { } func TestConnectionDoWatchOnceOnEmptyKey(t *testing.T) { - watchOnceNotSupported, err := test_helpers.IsTarantoolVersionLess(3, 0, 0) - if err != nil { - log.Fatalf("Could not check the Tarantool version: %s", err) - } - if watchOnceNotSupported { - return - } + test_helpers.SkipIfWatchOnceUnsupported(t) conn := test_helpers.ConnectWithValidation(t, dialer, opts) defer conn.Close() diff --git a/test_helpers/utils.go b/test_helpers/utils.go index e962dc619..5dba76ff7 100644 --- a/test_helpers/utils.go +++ b/test_helpers/utils.go @@ -200,6 +200,14 @@ func SkipIfWatchOnceUnsupported(t *testing.T) { SkipIfFeatureUnsupported(t, "watch once", 3, 0, 0) } +// SkipIfWatchOnceSupported skips test run if Tarantool with WatchOnce +// request type is used. +func SkipIfWatchOnceSupported(t *testing.T) { + t.Helper() + + SkipIfFeatureSupported(t, "watch once", 3, 0, 0) +} + // SkipIfCrudSpliceBroken skips test run if splice operation is broken // on the crud side. // https://github.com/tarantool/crud/issues/397