From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id DF71145C305
 for <tarantool-patches@dev.tarantool.org>;
 Sat,  5 Dec 2020 02:10:40 +0300 (MSK)
References: <27d8ef885dcb4726f4c7423e72a72038608a8628.1604361700.git.imeevma@gmail.com>
 <919343f4-214c-aa10-aec4-773ee510888a@tarantool.org>
 <20201203095423.GA204182@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <b93f02c9-bf2f-673a-ae6f-44d7224af501@tarantool.org>
Date: Sat, 5 Dec 2020 00:10:38 +0100
MIME-Version: 1.0
In-Reply-To: <20201203095423.GA204182@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v1 1/1] box: remove unnecessary
 rights from peristent functions
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Mergen Imeev <imeevma@tarantool.org>
Cc: tarantool-patches@dev.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