From 711756d727456bddd39ee52945dd5ae27a1a5684 Mon Sep 17 00:00:00 2001
From: Gleb Kashkin <g.kashkin@tarantool.org>
Date: Sat, 9 Sep 2023 08:49:26 +0000
Subject: [PATCH] config: postpone creds applier when in RO till RW

Before this patch credentials applier used to just skip if Tarantool
was in Read Only mode. Now it starts a fiber that waits for instance
to be switched to Read Write mode and then applies in the background.

Part of #8967

NO_DOC=documentation request will be filed manually for the whole
       credentials
---
 ...8967-creds-postope-sync-when-RO-till-RW.md |  5 ++
 src/box/lua/config/applier/credentials.lua    | 60 ++++++++++++-------
 .../credentials_applier_test.lua              | 56 +++++++++++++++++
 3 files changed, 100 insertions(+), 21 deletions(-)
 create mode 100644 changelogs/unreleased/gh-8967-creds-postope-sync-when-RO-till-RW.md

diff --git a/changelogs/unreleased/gh-8967-creds-postope-sync-when-RO-till-RW.md b/changelogs/unreleased/gh-8967-creds-postope-sync-when-RO-till-RW.md
new file mode 100644
index 0000000000..a7463e798a
--- /dev/null
+++ b/changelogs/unreleased/gh-8967-creds-postope-sync-when-RO-till-RW.md
@@ -0,0 +1,5 @@
+## feature/config
+
+* On read-only instances, Tarantool now synchronizes credentials
+  in the background when switching to read-write mode instead of
+  skipping the synchronization (gh-8967).
diff --git a/src/box/lua/config/applier/credentials.lua b/src/box/lua/config/applier/credentials.lua
index 03171f40f9..a5f278402e 100644
--- a/src/box/lua/config/applier/credentials.lua
+++ b/src/box/lua/config/applier/credentials.lua
@@ -1,5 +1,6 @@
 local log = require('internal.config.utils.log')
 local digest = require('digest')
+local fiber = require('fiber')
 
 --[[
 This intermediate representation is a formatted set of
@@ -466,33 +467,19 @@ end
 
 -- }}} Create users
 
-local function apply(config)
+local fiber_wait_rw
+
+local function set_credentials(config)
+    -- Credentials cannot be applied when instance is in Read Only state,
+    -- thus set_credentials() must be called only on Read Write instance.
+    assert(not box.info.ro)
+
     local configdata = config._configdata
     local credentials = configdata:get('credentials')
     if credentials == nil then
         return
     end
 
-    -- TODO: What if all the instances in a replicaset are
-    -- read-only at configuration applying? They all will ignore
-    -- the section and so it will never be applied -- even when
-    -- some instance goes to RW.
-    --
-    -- Moreover, this skip is silent: no log warnings or issue
-    -- reporting.
-    --
-    -- OTOH, a replica (downstream) should ignore all the config
-    -- data that is persisted.
-    --
-    -- A solution could be postpone applying of such data till
-    -- RW state. The applying should check that the data is not
-    -- added/updated already (arrived from master).
-    if box.info.ro then
-        log.verbose('credentials.apply: skip the credentials section, ' ..
-            'because the instance is in the read-only mode')
-        return
-    end
-
     -- Tarantool has the following roles and users present by default on every
     -- instance:
     --
@@ -526,6 +513,37 @@ local function apply(config)
     create_users(credentials.users)
 end
 
+local function wait_rw(config)
+    if box.info.ro then
+        log.verbose('credentials.apply: Tarantool is in Read Only mode ' ..
+                    'credentials will be set up in the background when it ' ..
+                    'is switched to Read Write mode')
+        box.ctl.wait_rw()
+    end
+    set_credentials(config)
+end
+
+local function apply(config)
+    -- Fiber is required here to have the credentials synced in the background
+    -- when Tarantool is in Read Only mode after it is switched to RW.
+
+    -- Note that only one fiber exists at a time.
+    if fiber_wait_rw == nil or fiber_wait_rw:status() == 'dead' then
+        fiber_wait_rw = fiber.new(wait_rw, config)
+    end
+
+    -- If Tarantool is already in Read Write mode, credentials are still
+    -- applied in the fiber to avoid possible concurrency issues, but the
+    -- main applier fiber is blocked by the `join()`.
+    if not box.info.ro then
+        fiber_wait_rw:set_joinable(true)
+        local ok, err = fiber_wait_rw:join()
+        if not ok then
+            error(err)
+        end
+    end
+end
+
 return {
     name = 'credentials',
     apply = apply,
diff --git a/test/config-luatest/credentials_applier_test.lua b/test/config-luatest/credentials_applier_test.lua
index 4ba7f28552..d4f146b1b7 100644
--- a/test/config-luatest/credentials_applier_test.lua
+++ b/test/config-luatest/credentials_applier_test.lua
@@ -727,3 +727,59 @@ g.test_restore_defaults_for_default_user = function(g)
         end,
     })
 end
+
+g.test_sync_ro_rw = function(g)
+    helpers.reload_success_case(g, {
+        options = {
+            credentials = {
+                users = {
+                    guest = {
+                        roles = { 'super' }
+                    },
+                }
+            }
+        },
+        verify = function() end,
+        options_2 = {
+            database = {
+                mode = 'ro',
+            },
+            credentials = {
+                roles = {
+                    dummy = {},
+                },
+                users = {
+                    guest = {
+                        roles = { 'super', 'dummy' }
+                    },
+                }
+            }
+        },
+        verify_2 = function()
+            t.assert(box.info.ro)
+
+            local internal =
+                    require('internal.config.applier.credentials')._internal
+
+            local perm = box.schema.user.info('guest')
+            perm = internal.privileges_from_box(perm)
+
+            t.assert_not_equals(perm['role']['dummy'], {execute = true})
+
+            box.cfg{read_only = false}
+            t.assert_not(box.info.ro)
+
+            local retrying = require('luatest.helpers').retrying
+
+            retrying(
+                {timeout = 10, delay = 0.5},
+                function()
+                    local perm = box.schema.user.info('guest')
+                    perm = internal.privileges_from_box(perm)
+
+                    t.assert_equals(perm['role']['dummy'], {execute = true})
+                end
+            )
+        end,
+    })
+end
-- 
GitLab