Skip to content

Commit

Permalink
config: fix wrong status after setting permissions
Browse files Browse the repository at this point in the history
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 tarantool#9689

NO_DOC=bugfix
NO_CHANGELOG=unreleased bug
  • Loading branch information
ImeevMA authored and Totktonada committed Feb 16, 2024
1 parent d08dbcb commit 38fc791
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/box/lua/config/applier/credentials.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/box/lua/config/utils/aboard.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
54 changes: 54 additions & 0 deletions test/config-luatest/credentials_applier_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 38fc791

Please sign in to comment.