From 345d52b84d4580eec1e090017b7918ba81c09e35 Mon Sep 17 00:00:00 2001
From: Alexander Turenko <alexander.turenko@tarantool.org>
Date: Tue, 27 Jun 2023 15:08:36 +0300
Subject: [PATCH] config: split iproto.advertise to several options

In brief:

* client -- for external clients
* peer -- for connections within the cluster, in particular for replicas
* sharding -- for routers and a rebalancer

See the instance_config.lua file for the details.

Part of #8810

NO_DOC=the old behavior was not released, the documentation request will
       be registered manually
NO_CHANGELOG=see NO_DOC
---
 doc/examples/config/etcd/local.yaml           |   4 +-
 doc/examples/config/replicaset.yaml           |   4 +-
 src/box/lua/config/applier/box_cfg.lua        |  30 ++---
 src/box/lua/config/instance_config.lua        | 117 +++++++++++++-----
 test/config-luatest/basic_test.lua            |  19 ++-
 .../cluster_config_schema_test.lua            |   6 +-
 test/config-luatest/helpers.lua               |   5 -
 .../instance_config_schema_test.lua           |  67 ++++++----
 test/luatest_helpers/server.lua               |  16 ++-
 9 files changed, 181 insertions(+), 87 deletions(-)

diff --git a/doc/examples/config/etcd/local.yaml b/doc/examples/config/etcd/local.yaml
index 2403d82eaa..432222250d 100644
--- a/doc/examples/config/etcd/local.yaml
+++ b/doc/examples/config/etcd/local.yaml
@@ -6,4 +6,6 @@ config:
 
 iproto:
   listen: 'unix/:./{{ instance_name }}.iproto'
-  advertise: 'replicator@'
+  advertise:
+    client: client@
+    peer: replicator@
diff --git a/doc/examples/config/replicaset.yaml b/doc/examples/config/replicaset.yaml
index 1d65e56415..869bca0815 100644
--- a/doc/examples/config/replicaset.yaml
+++ b/doc/examples/config/replicaset.yaml
@@ -11,7 +11,9 @@ credentials:
 
 iproto:
   listen: 'unix/:./{{ instance_name }}.iproto'
-  advertise: 'replicator@'
+  advertise:
+    client: client@
+    peer: replicator@
 
 log:
  to: file
diff --git a/src/box/lua/config/applier/box_cfg.lua b/src/box/lua/config/applier/box_cfg.lua
index 30ecf7f9a8..0ee254ac81 100644
--- a/src/box/lua/config/applier/box_cfg.lua
+++ b/src/box/lua/config/applier/box_cfg.lua
@@ -48,10 +48,12 @@ local function peer_uri(configdata, peer_name)
     local err_msg_prefix = ('box_cfg.apply: unable to build replicaset %q ' ..
         'of group %q'):format(names.replicaset_name, names.group_name)
 
-    local iproto = configdata:get('iproto', {peer = peer_name}) or {}
+    local opts = {peer = peer_name, use_default = true}
+    local listen = configdata:get('iproto.listen', opts)
+    local advertise = configdata:get('iproto.advertise.peer', opts)
 
-    if iproto.advertise ~= nil and not iproto.advertise:endswith('@') then
-        -- The iproto.advertise option contains an URI.
+    if advertise ~= nil and not advertise:endswith('@') then
+        -- The iproto.advertise.peer option contains an URI.
         --
         -- There are the following cases.
         --
@@ -67,44 +69,44 @@ local function peer_uri(configdata, peer_name)
         -- section of the config.
         --
         -- Otherwise, the URI is returned as is.
-        local u, err = urilib.parse(iproto.advertise)
+        local u, err = urilib.parse(advertise)
         -- NB: The URI is validated, so the parsing can't fail.
         assert(u ~= nil, err)
         if u.login ~= nil and u.password == nil then
             u.password = find_password(configdata, u.login)
             return urilib.format(u, true)
         end
-        return iproto.advertise
-    elseif iproto.listen ~= nil then
-        -- The iproto.advertise option has no URI.
+        return advertise
+    elseif listen ~= nil then
+        -- The iproto.advertise.peer option has no URI.
         --
         -- There are the following cases.
         --
-        -- * <no iproto.advertise>
+        -- * <no iproto.advertise.peer>
         -- * user@
         -- * user:pass@
         --
         -- In any case we should find an URI suitable to create a
         -- client socket in iproto.listen option. After this, add
         -- the auth information if any.
-        local uri = find_suitable_uri_to_connect(iproto.listen)
+        local uri = find_suitable_uri_to_connect(listen)
         if uri == nil then
-            error(('%s: instance %q has no iproto.advertise or ' ..
+            error(('%s: instance %q has no iproto.advertise.peer or ' ..
                 'iproto.listen URI suitable to create a client socket'):format(
                 err_msg_prefix, peer_name), 0)
         end
 
-        -- No additional auth information in iproto.advertise:
+        -- No additional auth information in iproto.advertise.peer:
         -- return the listen URI as is.
-        if iproto.advertise == nil then
+        if advertise == nil then
             return uri
         end
 
         -- Extract user and password from the iproto.advertise
         -- option. If no password given, find it in the
         -- 'credentials' section of the config.
-        assert(iproto.advertise:endswith('@'))
-        local auth = iproto.advertise:sub(1, -2):split(':', 1)
+        assert(advertise:endswith('@'))
+        local auth = advertise:sub(1, -2):split(':', 1)
         local username = auth[1]
         local password = auth[2] or find_password(configdata, username)
 
diff --git a/src/box/lua/config/instance_config.lua b/src/box/lua/config/instance_config.lua
index d8aa34f2a2..36f278a800 100644
--- a/src/box/lua/config/instance_config.lua
+++ b/src/box/lua/config/instance_config.lua
@@ -160,6 +160,35 @@ local function uri_is_suitable_to_connect(_, uri)
     return true
 end
 
+local function advertise_uri_validate(data, w)
+    -- Accept the special syntax user@ or user:pass@.
+    --
+    -- It means using iproto.listen value to connect and using the
+    -- given user and password.
+    --
+    -- If the password is not given, it is extracted from the
+    -- `credentials` section of the config.
+    if data:endswith('@') then
+        return
+    end
+
+    -- Substitute variables with placeholders to don't confuse the
+    -- URI parser with the curly brackets.
+    data = data:gsub('{{ *.- *}}', 'placeholder')
+
+    local uri, err = urilib.parse(data)
+    if uri == nil then
+        if data:find(',') then
+            w.error('A single URI is expected, not a list of URIs')
+        end
+        w.error('Unable to parse an URI: %s', err)
+    end
+    local ok, err = uri_is_suitable_to_connect(nil, uri)
+    if not ok then
+        w.error(err)
+    end
+end
+
 return schema.new('instance_config', schema.record({
     config = schema.record({
         version = schema.enum({
@@ -446,37 +475,63 @@ return schema.new('instance_config', schema.record({
                 end
             end,
         }),
-        advertise = schema.scalar({
-            type = 'string',
-            default = box.NULL,
-            validate = function(data, w)
-                -- Accept the special syntax user@ or user:pass@.
-                --
-                -- It means using iproto.listen value to connect
-                -- and using the given user and password.
-                --
-                -- If the password is not given, it is extracted
-                -- from the `credentials` section of the config.
-                if data:endswith('@') then
-                    return
-                end
-
-                -- Substitute variables with placeholders to don't
-                -- confuse the URI parser with the curly brackets.
-                data = data:gsub('{{ *.- *}}', 'placeholder')
-
-                local uri, err = urilib.parse(data)
-                if uri == nil then
-                    if data:find(',') then
-                        w.error('A single URI is expected, not a list of URIs')
-                    end
-                    w.error('Unable to parse an URI: %s', err)
-                end
-                local ok, err = uri_is_suitable_to_connect(nil, uri)
-                if not ok then
-                    w.error(err)
-                end
-            end,
+        -- URIs for clients to let them know where to connect.
+        --
+        -- There are several possibly different URIs:
+        --
+        -- * client
+        --
+        --   The general purpose client URI, usually points to a
+        --   restricted user with access to particular functions.
+        --
+        -- * peer
+        --
+        --   The general purpose peer URI, used for connections
+        --   within the cluster (replica -> master, router ->
+        --   storage, rebalancer -> storage).
+        --
+        --   Usually points to a user with the 'replication' role.
+        --
+        -- * sharding
+        --
+        --   The URI for router and rebalancer.
+        --
+        --   If unset, the general peer URI should be used.
+        --
+        -- All the iproto.advertise.* options have the following
+        -- syntax variants:
+        --
+        -- 1. user@
+        -- 2. user:pass@
+        -- 3. user@host:port
+        -- 4. host:port
+        --
+        -- Note: the host:port part may represent a Unix domain
+        -- socket: host = 'unix/', port = '/path/to/socket'.
+        --
+        -- If there is no host:port (1, 2), it is to be looked in
+        -- iproto.listen.
+        --
+        -- If there is a user, but no password (1, 3), the
+        -- password is to be looked in the `credentials` section
+        -- of the configuration (except user 'guest', which can't
+        -- have a password).
+        advertise = schema.record({
+            client = schema.scalar({
+                type = 'string',
+                default = box.NULL,
+                validate = advertise_uri_validate,
+            }),
+            peer = schema.scalar({
+                type = 'string',
+                default = box.NULL,
+                validate = advertise_uri_validate,
+            }),
+            sharding = schema.scalar({
+                type = 'string',
+                default = box.NULL,
+                validate = advertise_uri_validate,
+            }),
         }),
         threads = schema.scalar({
             type = 'integer',
diff --git a/test/config-luatest/basic_test.lua b/test/config-luatest/basic_test.lua
index 23f23f8144..ecad6a1b7e 100644
--- a/test/config-luatest/basic_test.lua
+++ b/test/config-luatest/basic_test.lua
@@ -81,7 +81,8 @@ local err_msg_cannot_find_user = 'box_cfg.apply: cannot find user unknown ' ..
     'in the config to use its password in a replication peer URI'
 local err_msg_no_suitable_uris = 'box_cfg.apply: unable to build replicaset ' ..
     '"replicaset-001" of group "group-001": instance "instance-002" has no ' ..
-    'iproto.advertise or iproto.listen URI suitable to create a client socket'
+    'iproto.advertise.peer or iproto.listen URI suitable to create a client ' ..
+    'socket'
 
 -- Bad cases for building replicaset URIs from iproto.advertise
 -- and iproto.listen parameters.
@@ -124,7 +125,9 @@ for case_name, case in pairs({
         local instance_002 = {
             iproto = {
                 listen = case.listen,
-                advertise = case.advertise,
+                advertise = {
+                    peer = case.advertise,
+                },
             },
         }
         local good_listen = 'unix/:./{{ instance_name }}.iproto'
@@ -153,14 +156,18 @@ for case_name, case in pairs({
                                     },
                                     iproto = {
                                         listen = good_listen,
-                                        advertise = 'replicator@',
+                                        advertise = {
+                                            peer = 'replicator@',
+                                        },
                                     },
                                 },
                                 ['instance-002'] = instance_002,
                                 ['instance-003'] = {
                                     iproto = {
                                         listen = good_listen,
-                                        advertise = 'replicator@',
+                                        advertise = {
+                                            peer = 'replicator@',
+                                        },
                                     },
                                 },
                             },
@@ -250,7 +257,9 @@ for case_name, case in pairs({
             },
             iproto = {
                 listen = case.listen,
-                advertise = case.advertise,
+                advertise = {
+                    peer = case.advertise,
+                },
             },
             groups = {
                 ['group-001'] = {
diff --git a/test/config-luatest/cluster_config_schema_test.lua b/test/config-luatest/cluster_config_schema_test.lua
index 0ab9152539..c83ff3ec7b 100644
--- a/test/config-luatest/cluster_config_schema_test.lua
+++ b/test/config-luatest/cluster_config_schema_test.lua
@@ -141,7 +141,11 @@ g.test_defaults = function()
         },
         iproto = {
             listen = box.NULL,
-            advertise = box.NULL,
+            advertise = {
+                client = box.NULL,
+                peer = box.NULL,
+                sharding = box.NULL,
+            },
             threads = 1,
             net_msg_max = 768,
             readahead = 16320,
diff --git a/test/config-luatest/helpers.lua b/test/config-luatest/helpers.lua
index b796ceec49..24d45ec390 100644
--- a/test/config-luatest/helpers.lua
+++ b/test/config-luatest/helpers.lua
@@ -3,14 +3,9 @@ local t = require('luatest')
 local server = require('test.luatest_helpers.server')
 
 local function start_example_replicaset(g, dir, config_file, opts)
-    local credentials = {
-        user = 'client',
-        password = 'secret',
-    }
     local opts = fun.chain({
         config_file = config_file,
         chdir = dir,
-        net_box_credentials = credentials,
     }, opts or {}):tomap()
     g.server_1 = server:new(fun.chain(opts, {alias = 'instance-001'}):tomap())
     g.server_2 = server:new(fun.chain(opts, {alias = 'instance-002'}):tomap())
diff --git a/test/config-luatest/instance_config_schema_test.lua b/test/config-luatest/instance_config_schema_test.lua
index 71df07f7ac..06fa1f4a5c 100644
--- a/test/config-luatest/instance_config_schema_test.lua
+++ b/test/config-luatest/instance_config_schema_test.lua
@@ -290,7 +290,11 @@ g.test_iproto = function()
     local iconfig = {
         iproto = {
             listen = 'one',
-            advertise = 'two',
+            advertise = {
+                client = 'two',
+                peer = 'three',
+                sharding = 'four',
+            },
             threads = 1,
             net_msg_max = 1,
             readahead = 1,
@@ -301,7 +305,11 @@ g.test_iproto = function()
 
     local exp = {
         listen = box.NULL,
-        advertise = box.NULL,
+        advertise = {
+            client = box.NULL,
+            peer = box.NULL,
+            sharding = box.NULL,
+        },
         threads = 1,
         net_msg_max = 768,
         readahead = 16320,
@@ -315,7 +323,7 @@ for case_name, case in pairs({
     incorrect_uri = {
         advertise = ':3301',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'Unable to parse an URI',
             'Incorrect URI',
             'expected host:service or /unix.socket'
@@ -324,82 +332,87 @@ for case_name, case in pairs({
     multiple_uris = {
         advertise = 'localhost:3301,localhost:3302',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'A single URI is expected, not a list of URIs',
         }, ': '),
     },
     inaddr_any_ipv4 = {
         advertise = '0.0.0.0:3301',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'INADDR_ANY (0.0.0.0) cannot be used to create a client socket',
         }, ': '),
     },
     inaddr_any_ipv4_user = {
         advertise = 'user@0.0.0.0:3301',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'INADDR_ANY (0.0.0.0) cannot be used to create a client socket',
         }, ': '),
     },
     inaddr_any_ipv4_user_pass = {
         advertise = 'user:pass@0.0.0.0:3301',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'INADDR_ANY (0.0.0.0) cannot be used to create a client socket',
         }, ': '),
     },
     inaddr_any_ipv6 = {
         advertise = '[::]:3301',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'in6addr_any (::) cannot be used to create a client socket',
         }, ': '),
     },
     inaddr_any_ipv6_user = {
         advertise = 'user@[::]:3301',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'in6addr_any (::) cannot be used to create a client socket',
         }, ': '),
     },
     inaddr_any_ipv6_user_pass = {
         advertise = 'user:pass@[::]:3301',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'in6addr_any (::) cannot be used to create a client socket',
         }, ': '),
     },
     zero_port = {
         advertise = 'localhost:0',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'An URI with zero port cannot be used to create a client socket',
         }, ': '),
     },
     zero_port_user = {
         advertise = 'user@localhost:0',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'An URI with zero port cannot be used to create a client socket',
         }, ': '),
     },
     zero_port_user_pass = {
         advertise = 'user:pass@localhost:0',
         exp_err_msg = table.concat({
-            '[instance_config] iproto.advertise',
+            '[instance_config] iproto.advertise.%s',
             'An URI with zero port cannot be used to create a client socket',
         }, ': '),
     },
 }) do
     g[('test_bad_iproto_advertise_%s'):format(case_name)] = function()
-        t.assert_error_msg_equals(case.exp_err_msg, function()
-            instance_config:validate({
-                iproto = {
-                    advertise = case.advertise,
-                },
-            })
-        end)
+        for _, option_name in ipairs({'client', 'peer', 'sharding'}) do
+            local exp_err_msg = case.exp_err_msg:format(option_name)
+            t.assert_error_msg_equals(exp_err_msg, function()
+                instance_config:validate({
+                    iproto = {
+                        advertise = {
+                            [option_name] = case.advertise,
+                        },
+                    },
+                })
+            end)
+        end
     end
 end
 
@@ -432,11 +445,15 @@ for case_name, case in pairs({
 }) do
     g[('test_good_iproto_advertise_%s'):format(case_name)] = function()
         assert(case.advertise ~= nil)
-        instance_config:validate({
-            iproto = {
-                advertise = case.advertise,
-            },
-        })
+        for _, option_name in ipairs({'client', 'peer', 'sharding'}) do
+            instance_config:validate({
+                iproto = {
+                    advertise = {
+                        [option_name] = case.advertise,
+                    },
+                },
+            })
+        end
     end
 end
 
diff --git a/test/luatest_helpers/server.lua b/test/luatest_helpers/server.lua
index cb892e3f8a..229ef349d1 100644
--- a/test/luatest_helpers/server.lua
+++ b/test/luatest_helpers/server.lua
@@ -58,22 +58,30 @@ local function find_advertise_uri(config, instance_name, dir)
                 break
             end
             if instance.iproto ~= nil then
-                advertise = advertise or instance.iproto.advertise
+                if instance.iproto.advertise ~= nil then
+                    advertise = advertise or instance.iproto.advertise.client
+                end
                 listen = listen or instance.iproto.listen
             end
             if replicaset.iproto ~= nil then
-                advertise = advertise or replicaset.iproto.advertise
+                if replicaset.iproto.advertise ~= nil then
+                    advertise = advertise or replicaset.iproto.advertise.client
+                end
                 listen = listen or replicaset.iproto.listen
             end
             if group.iproto ~= nil then
-                advertise = advertise or group.iproto.advertise
+                if group.iproto.advertise ~= nil then
+                    advertise = advertise or group.iproto.advertise.client
+                end
                 listen = listen or group.iproto.listen
             end
         end
     end
 
     if config.iproto ~= nil then
-        advertise = advertise or config.iproto.advertise
+        if config.iproto.advertise ~= nil then
+            advertise = advertise or config.iproto.advertise.client
+        end
         listen = listen or config.iproto.listen
     end
 
-- 
GitLab