Re: [Tarantool-patches] [PATCH v1 1/1] box: remove unnecessary rights from peristent functions

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the patch!

It looks good except for one comment below. Please, proceed to
making it work on all versions, like we discussed.

> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index add791cd7..b2475b0f6 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -971,6 +971,36 @@ local function upgrade_to_2_3_1()
>      create_session_settings_space()
>  end
>  
> +--------------------------------------------------------------------------------
> +-- Tarantool 2.7.1
> +--------------------------------------------------------------------------------
> +local function backport_upgrade_2_7_1_function_access()
> +    local _func = box.space._func
> +    local _priv = box.space._priv
> +    local datetime = os.date("%Y-%m-%d %H:%M:%S")
> +    local funcs_to_change = {'LUA', 'box.schema.user.info'}
> +    for _, name in pairs(funcs_to_change) do
> +        local func = _func.index['name']:get(name)
> +        -- Change setuid of function function if it is not 0.

"function function"? Tbh, the entire comment looks unneeded. It
just literally narrates the condition check. If you want to have
a comment here, better explain what is wrong with having setuid
set. Or nothing.

> +        if func ~= nil and func.setuid ~= 0 then
> +            local id = func.id
> +            log.info('remove old function "'..name..'"')
> +            _priv:delete({2, 'function', id})
> +            _func:delete({id})
> +            log.info('create function "'..name..'" with unset setuid')
> +            local new_func = func:update({{'=', 4, 0}, {'=', 18, datetime},
> +                                          {'=', 19, datetime}})
> +            _func:replace(new_func)
> +            log.info('grant execute on function "'..name..'" to public')
> +            _priv:replace{ADMIN, PUBLIC, 'function', id, box.priv.X}
> +        end
> +    end
> +end
> +
> +local function upgrade_to_2_7_1()
> +    backport_upgrade_2_7_1_function_access()
> +end