Skip to content

Commit

Permalink
config: delayed privilege alert doesn't stuck now
Browse files Browse the repository at this point in the history
The declarative configuration has the `credentials` section that
describes users and their privileges. It is OK to have privileges for a
space/function/sequence that does not exist. Such a privilege will lead
to an alert that states that the privilege will be granted, when the
object will be created.

The problem that is fixed by this commit is that such an alert was not
dropped, when the object is created and the relevant privileges are
granted.

Updated several unit test cases, because the credentials applier now
depends more on the `config` module: successful privilege grant calls
`config._aboard:drop()`.

Fixes tarantool#9574

NO_DOC=bugfix
  • Loading branch information
Totktonada committed Jan 25, 2024
1 parent 25deb4a commit c701892
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## bugfix/config

* An alert regarding delayed privilege granting is now cleared when the
privilege is granted (gh-9574).
84 changes: 82 additions & 2 deletions src/box/lua/config/applier/credentials.lua
Original file line number Diff line number Diff line change
Expand Up @@ -525,17 +525,94 @@ end
local privileges_action_f = function(grant_or_revoke, role_or_user, name, privs,
obj_type, obj_name)
assert(grant_or_revoke == 'grant' or grant_or_revoke == 'revoke')
privs = table.copy(privs)
table.sort(privs)
privs = table.concat(privs, ',')

-- Try to apply the action immediately. If the object doesn't exist,
-- the sync will be applied inside the trigger on object creation/rename.
local ok, err = pcall(box.schema[role_or_user][grant_or_revoke],
name, privs, obj_type, obj_name)

-- This key identifies an alert to be dropped on a successful
-- operation and to be set on an error.
--
-- Let's consider the following situation: a space does not
-- exist, but the configuration contains privileges for it
-- (for example 'read' and 'write'). The warning will be
-- issued with the "box.schema.user.grant(<...>, 'read,write',
-- <...>)" key.
--
-- Now, let's assume that the space is created. We should drop
-- the warning and, so, we should generate exactly same key to
-- pass to the :drop() aboard method.
--
-- 1. Is it possible that the key to drop contains just 'read',
-- not 'read,write'? Seems unlikely.
--
-- If the configuration was changed before the space
-- creation, then the old ('read,write') alert is dropped
-- together with all the alerts from the old configuration
-- and the new alert ('read') is issued on applying the new
-- configuration.
--
-- OTOH, if a space creation is in the same transaction
-- with granting a 'write' permission, it seems possible.
-- However, it looks unlikely that permissions for the same
-- user and the same object (space/function/sequence) are
-- granted using box.schema.user.grant() and using the
-- declarative configuration at the same time.
--
-- 2. Is it possible that separate calls with 'read' and
-- 'write' will be performed? Seems, no.
--
-- privileges_from_config() glues all the config privileges
-- into a mapping, so different configuration forms do not
-- affect the result. The following two configurations are
-- equivalent.
--
-- credentials:
-- users:
-- myuser:
-- privileges:
-- - permissions: [read, write]
-- spaces: [s]
--
-- credentials:
-- users:
-- myuser:
-- privileges:
-- - permissions: [read]
-- spaces: [s]
-- - permissions: [write]
-- spaces: [s]
--
-- 3. Is it possible that 'write,read' is in the key to drop,
-- while 'read,write' is in the original alert? No.
--
-- Even if we got a different order as input of this
-- function, it is sorted before the key generation: see
-- table.sort() above.
--
-- What to do if we found an inconsistency that leads to one
-- of the three situations above?
--
-- The simplest solution is to issue several alerts for each
-- object: one for the 'read' permission, one for 'write'
-- permission and so on.
--
-- More complicated solution is to perform a traverse for
-- relevant alerts on successful privilege granting to modify
-- or drop a right alert. In this case we don't know an exact
-- list of privileges to form a key to drop the alert.
local key = ('box.schema.%s.%s(%q, %q, %q, %q)'):format(
role_or_user, grant_or_revoke, name, privs, obj_type, obj_name)

if ok then
log.verbose('credentials.apply: box.schema.%s.%s(%q, %q, %q, %q)',
role_or_user, grant_or_revoke, name, privs,
obj_type, obj_name)
config._aboard:drop(key)
return
end
if err.code ~= box.error.NO_SUCH_SPACE and
Expand All @@ -544,13 +621,13 @@ local privileges_action_f = function(grant_or_revoke, role_or_user, name, privs,
err = ('credentials.apply: box.schema.%s.%s(%q, %q, %q, %q) failed: %s')
:format(role_or_user, grant_or_revoke, name, privs, obj_type,
obj_name, err)
config._aboard:set({type = 'error', message = err})
config._aboard:set({type = 'error', message = err}, {key = key})
else
local msg = "credentials.apply: %s %q hasn't been created yet, " ..
"'box.schema.%s.%s(%q, %q, %q, %q)' will be applied later"
msg = msg:format(obj_type, obj_name, role_or_user, grant_or_revoke,
name, privs, obj_type, obj_name)
config._aboard:set({type = 'warn', message = msg})
config._aboard:set({type = 'warn', message = msg}, {key = key})
end
end

Expand Down Expand Up @@ -908,6 +985,9 @@ return {
apply = apply,
-- Exported for testing purposes.
_internal = {
set_config = function(config_module)
config = config_module
end,
privileges_from_box = privileges_from_box,
privileges_from_config = privileges_from_config,
privileges_subtract = privileges_subtract,
Expand Down
6 changes: 5 additions & 1 deletion test/config-luatest/appliers_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ local g = t.group()
local appliers_script = [[
local configdata = require('internal.config.configdata')
local cluster_config = require('internal.config.cluster_config')
local aboard = require('internal.config.utils.aboard')
local cconfig = {
credentials = {
users = {
Expand Down Expand Up @@ -40,7 +41,10 @@ local appliers_script = [[
},
}
local iconfig = cluster_config:instantiate(cconfig, 'instance-001')
config = {_configdata = configdata.new(iconfig, cconfig, 'instance-001')}
config = {
_configdata = configdata.new(iconfig, cconfig, 'instance-001'),
_aboard = aboard.new(),
}
local mkdir = require('internal.config.applier.mkdir')
mkdir.apply(config)
local box_cfg = require('internal.config.applier.box_cfg')
Expand Down
56 changes: 54 additions & 2 deletions test/config-luatest/credentials_applier_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,10 @@ g.test_sync_privileges = function(g)
child:roundtrip(("box.schema.user.%s(%q, %q, %q, %q, %s)"):format(
action, name, perm, obj_type, obj_name, opts))
end
child:roundtrip("sync_privileges = require('internal.config.applier." ..
"credentials')._internal.sync_privileges")
child:roundtrip("applier = require('internal.config.applier.credentials')")
child:roundtrip("applier._internal.set_config({_aboard = " ..
"require('internal.config.utils.aboard').new()})")
child:roundtrip("sync_privileges = applier._internal.sync_privileges")
child:roundtrip("json = require('json')")
child:roundtrip(("credentials = json.decode(%q)"):format(
json.encode(credentials)))
Expand Down Expand Up @@ -1400,3 +1402,53 @@ g.test_delayed_grant_alert = function(g)
end,
})
end

-- Verify that an alert regarding a delayed privilege granting
-- (due to lack of a space/function/sequence) is dropped, when the
-- privilege is finally granted.
--
-- This test case starts an instance with a configuration that
-- contains permissions on space 's'. The startup has the
-- following steps.
--
-- 1. The new database is up.
-- 2. The credentials applier sees the privileges and sees that
-- space 's' does not exist. It issues an alert.
-- 3. The application script is started and it creates the space.
-- 4. The credentials applier wakes up (it has a trigger), grants
-- the requested privileges and eliminates the alert that is
-- issued on the step 2.
--
-- This test case verifies that the last step actually eliminates
-- the alert.
g.test_delayed_grant_alert_dropped = function(g)
helpers.success_case(g, {
script = string.dump(function()
box.once('app', function()
box.schema.space.create('s')
box.space.s:create_index('pk')
end)
end),
options = {
['app.file'] = 'main.lua',
['credentials.users.guest.privileges'] = {
{
permissions = {'read', 'write'},
spaces = {'s'},
},
},
},
verify = function()
local config = require('config')

local info = config:info()
t.assert_equals({
status = info.status,
alerts = info.alerts,
}, {
status = 'ready',
alerts = {},
})
end,
})
end

0 comments on commit c701892

Please sign in to comment.