From 38fc791cbf50615d6c9a3fb2de23b924449f287c Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@tarantool.org>
Date: Wed, 14 Feb 2024 14:21:29 +0300
Subject: [PATCH] config: fix wrong status after setting permissions

Before this patch, it was possible that the config status could change
to 'ready' even if there were pending alerts. This could happen if there
were 'missed_privilege' warnings. Before this patch, warnings were
cleared and then refilled if necessary. However, after clearing all
these warnings, if there were no other warnings, the status was set to
'ready'. And if new warnings were added during the 'refilling' stage,
the status did not change from 'ready' to 'check_warnings' or
'check_errors'.

Part of #9689

NO_DOC=bugfix
NO_CHANGELOG=unreleased bug
---
 src/box/lua/config/applier/credentials.lua    | 17 ++++--
 src/box/lua/config/utils/aboard.lua           |  8 +++
 .../credentials_applier_test.lua              | 54 +++++++++++++++++++
 3 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/src/box/lua/config/applier/credentials.lua b/src/box/lua/config/applier/credentials.lua
index 72e26028c450..8f5e340dd4de 100644
--- a/src/box/lua/config/applier/credentials.lua
+++ b/src/box/lua/config/applier/credentials.lua
@@ -559,16 +559,20 @@ local function sync_privileges(credentials, obj_to_sync)
                     obj_to_sync.type, obj_to_sync.name)
     end
 
-    -- Drop missed privilege alerts.
+    -- Prepare to drop missed privilege alerts.
     --
     -- The actual alerts will be issued in the sync() function
     -- below.
     --
     -- Important: we don't follow the `obj_to_sync` filter here
     -- for simplicity. Just revisit all the missed privilege
-    -- alerts: drop all of them and issue again the actual ones.
-    config._aboard:drop_if(function(_key, alert)
-        return alert._trait == 'missed_privilege'
+    -- alerts: mark all of them and issue again the actual ones.
+    -- The actual drop will occur later. New alerts, if any, are
+    -- issued before the drop.
+    config._aboard:each(function(_key, alert)
+        if alert._trait == 'missed_privilege' then
+            alert._trait = 'missed_privilege_obsolete'
+        end
     end)
 
     -- The privileges synchronization between A and B is performed in 3 steps:
@@ -676,6 +680,11 @@ local function sync_privileges(credentials, obj_to_sync)
     for name, role_def in pairs(credentials.roles or {}) do
         sync('role', name, role_def)
     end
+
+    -- Drop obsolete missed_privilege alerts.
+    config._aboard:drop_if(function(_key, alert)
+        return alert._trait == 'missed_privilege_obsolete'
+    end)
 end
 
 -- }}} Main sync logic
diff --git a/src/box/lua/config/utils/aboard.lua b/src/box/lua/config/utils/aboard.lua
index 49a244394c40..1b8534169e8c 100644
--- a/src/box/lua/config/utils/aboard.lua
+++ b/src/box/lua/config/utils/aboard.lua
@@ -92,6 +92,13 @@ local function aboard_drop_if(self, filter_f)
     end
 end
 
+-- Traverse all alerts and apply the provided callback function to each of them.
+local function aboard_each(self, callback)
+    for key, alert in pairs(self._alerts) do
+        callback(key, alert)
+    end
+end
+
 -- Drop all the alerts.
 --
 -- The `on_drop` callback is NOT called.
@@ -156,6 +163,7 @@ local mt = {
     __index = {
         set = aboard_set,
         get = aboard_get,
+        each = aboard_each,
         drop = aboard_drop,
         drop_if = aboard_drop_if,
         clean = aboard_clean,
diff --git a/test/config-luatest/credentials_applier_test.lua b/test/config-luatest/credentials_applier_test.lua
index d97a51bb9a40..b58b47ce2604 100644
--- a/test/config-luatest/credentials_applier_test.lua
+++ b/test/config-luatest/credentials_applier_test.lua
@@ -1712,3 +1712,57 @@ g.test_space_rename_rw2r = function(g)
         })
     end)
 end
+
+-- Verify that the config status does not change if there are pending
+-- alerts when some missed_privilege alerts are dropped.
+g.test_fix_status_change_with_pending_warnings = function(g)
+    -- Create a configuration with privileges for two missing spaces.
+    local config = cbuilder.new()
+        :add_instance('i-001', {})
+        :set_global_option('credentials.users.guest.privileges', {
+            {
+                permissions = {'read'},
+                spaces = {'one', 'two'},
+            },
+        })
+        :config()
+
+    local replicaset = replicaset.new(g, config)
+    replicaset:start()
+    define_warnings_function(replicaset['i-001'])
+
+    -- Verify that alerts are set for delayed privilege grants
+    -- for the given spaces and status is 'check_warnings'.
+    replicaset['i-001']:exec(function()
+        t.assert_equals(_G.warnings(), {
+            'one read',
+            'two read',
+        })
+        t.assert_equals(require('config'):info().status, 'check_warnings')
+    end)
+
+    -- Create space 'one'.
+    replicaset['i-001']:exec(function()
+        box.schema.space.create('one')
+    end)
+
+    -- Verify that the alerts regarding the space 'one' are gone,
+    -- but the status is still 'check_warnings'.
+    replicaset['i-001']:exec(function()
+        t.assert_equals(_G.warnings(), {
+            'two read',
+        })
+        t.assert_equals(require('config'):info().status, 'check_warnings')
+    end)
+
+    -- Create space 'two'.
+    replicaset['i-001']:exec(function()
+        box.schema.space.create('two')
+    end)
+
+    -- Verify than all alerts are gone and the status is 'ready'.
+    replicaset['i-001']:exec(function()
+        t.assert_equals(_G.warnings(), {})
+        t.assert_equals(require('config'):info().status, 'ready')
+    end)
+end