From 38f88795d2894f08e6b9af67bc7eaa9a87a414a2 Mon Sep 17 00:00:00 2001
From: Ilya Verbin <iverbin@tarantool.org>
Date: Mon, 10 Oct 2022 18:01:39 +0300
Subject: [PATCH] box: forbid DDL operations until box.schema.upgrade

Currently, in case of recovery from an old snapshot, Tarantool allows to
perform DDL operations on an instance with non-upgraded schema.
It leads to various unpredictable errors (because the DDL code assumes
that the schema is already upgraded). This patch forbids the following
operations unless the user has the most recent schema version:
- box.schema.space.create
- box.schema.space.drop
- box.schema.space.alter
- box.schema.index.create
- box.schema.index.drop
- box.schema.index.alter
- box.schema.sequence.create
- box.schema.sequence.drop
- box.schema.sequence.alter
- box.schema.func.create
- box.schema.func.drop

Closes #7149

NO_DOC=bugfix
---
 ...149-forbid-ddl-until-box-schema-upgrade.md |   3 +
 src/box/errcode.h                             |   1 +
 src/box/lua/load_cfg.lua                      |   6 +-
 src/box/lua/schema.lua                        |  19 ++++++
 src/box/lua/upgrade.lua                       |  28 ++++++---
 ...rbid_ddl_until_box_schema_upgrade_test.lua |  59 ++++++++++++++++++
 .../upgrade/1.10/00000000000000000004.snap    | Bin 0 -> 1612 bytes
 test/box/error.result                         |   1 +
 8 files changed, 104 insertions(+), 13 deletions(-)
 create mode 100644 changelogs/unreleased/gh-7149-forbid-ddl-until-box-schema-upgrade.md
 create mode 100644 test/box-luatest/upgrade/1.10/00000000000000000004.snap

diff --git a/changelogs/unreleased/gh-7149-forbid-ddl-until-box-schema-upgrade.md b/changelogs/unreleased/gh-7149-forbid-ddl-until-box-schema-upgrade.md
new file mode 100644
index 0000000000..1cce7e07d2
--- /dev/null
+++ b/changelogs/unreleased/gh-7149-forbid-ddl-until-box-schema-upgrade.md
@@ -0,0 +1,3 @@
+## bugfix/core
+
+* Forbidden DDL operations for the non-upgraded schema (gh-7149).
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 5b72c1879b..4807745830 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -300,6 +300,7 @@ struct errcode_record {
 	/*245 */_(ER_OLD_TERM,			"The term is outdated: old - %llu, new - %llu") \
 	/*246 */_(ER_INTERFERING_ELECTIONS,	"Interfering elections started")\
 	/*247 */_(ER_ITERATOR_POSITION,		"Iterator position is invalid") \
+	/*248 */_(ER_DDL_NOT_ALLOWED,		"DDL operations are not allowed: %s") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 65587f7843..ad5e4908b8 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -979,12 +979,8 @@ local function load_cfg(cfg)
     -- Check if schema version matches Tarantool version and print
     -- warning if it's not (in case user forgot to call
     -- box.schema.upgrade()).
-    local needs, schema_version_str = private.schema_needs_upgrade()
+    local needs, msg = private.schema_needs_upgrade()
     if needs then
-        local msg = string.format(
-            'Your schema version is %s while Tarantool %s requires a more'..
-            ' recent schema version. Please, consider using box.'..
-            'schema.upgrade().', schema_version_str, box.info.version)
         log.warn(msg)
     end
 end
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index c12e9d2898..992effa4ea 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -241,6 +241,14 @@ local function revoke_object_privs(object_type, object_id)
     end
 end
 
+-- Check if schema version matches Tarantool version and raise an error if not.
+local function check_schema_version()
+    local needs_upgrade, msg = internal.schema_needs_upgrade()
+    if needs_upgrade then
+        box.error(box.error.DDL_NOT_ALLOWED, msg)
+    end
+end
+
 -- Same as type(), but returns 'number' if 'param' is
 -- of type 'cdata' and represents a 64-bit integer.
 local function param_type(param)
@@ -797,6 +805,7 @@ box.schema.space.create = function(name, options)
         temporary = false,
     }
     check_param_table(options, options_template)
+    check_schema_version()
     options = update_param_table(options, options_defaults)
     if options.engine == 'vinyl' then
         options = update_param_table(options, {
@@ -885,6 +894,7 @@ box.schema.space.drop = function(space_id, space_name, opts)
     check_param(space_id, 'space_id', 'number')
     opts = opts or {}
     check_param_table(opts, { if_exists = 'boolean' })
+    check_schema_version()
     local _space = box.space[box.schema.SPACE_ID]
     local _index = box.space[box.schema.INDEX_ID]
     local _trigger = box.space[box.schema.TRIGGER_ID]
@@ -956,6 +966,7 @@ box.schema.space.alter = function(space_id, options)
         box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id))
     end
     check_param_table(options, alter_space_template)
+    check_schema_version()
 
     local _space = box.space._space
     local tuple = _space:get({space.id})
@@ -1527,6 +1538,7 @@ box.schema.index.create = function(space_id, name, options)
     check_param(space_id, 'space_id', 'number')
     check_param(name, 'name', 'string')
     check_param_table(options, create_index_template)
+    check_schema_version()
     local space = box.space[space_id]
     if not space then
         box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id))
@@ -1656,6 +1668,7 @@ end
 box.schema.index.drop = function(space_id, index_id)
     check_param(space_id, 'space_id', 'number')
     check_param(index_id, 'index_id', 'number')
+    check_schema_version()
     if index_id == 0 then
         local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
         local sequence_tuple = _space_sequence:delete{space_id}
@@ -1696,6 +1709,7 @@ box.schema.index.alter = function(space_id, index_id, options)
     end
 
     check_param_table(options, alter_index_template)
+    check_schema_version()
 
     if type(space_id) ~= "number" then
         space_id = space.id
@@ -2885,6 +2899,7 @@ box.schema.sequence.create = function(name, opts)
     opts = opts or {}
     check_param(name, 'name', 'string')
     check_param_table(opts, create_sequence_options)
+    check_schema_version()
     local ascending = not opts.step or opts.step > 0
     local options_defaults = {
         step = 1,
@@ -2910,6 +2925,7 @@ end
 
 box.schema.sequence.alter = function(name, opts)
     check_param_table(opts, alter_sequence_options)
+    check_schema_version()
     local id, tuple = sequence_resolve(name)
     if id == nil then
         box.error(box.error.NO_SUCH_SEQUENCE, name)
@@ -2929,6 +2945,7 @@ end
 box.schema.sequence.drop = function(name, opts)
     opts = opts or {}
     check_param_table(opts, {if_exists = 'boolean'})
+    check_schema_version()
     local id = sequence_resolve(name)
     if id == nil then
         if not opts.if_exists then
@@ -3171,6 +3188,7 @@ box.schema.func.create = function(name, opts)
                               comment = 'string',
                               param_list = 'table', returns = 'string',
                               exports = 'table', opts = 'table' })
+    check_schema_version()
     local _func = box.space[box.schema.FUNC_ID]
     local _vfunc = box.space[box.schema.VFUNC_ID]
     local func = _vfunc.index.name:get{name}
@@ -3206,6 +3224,7 @@ end
 box.schema.func.drop = function(name, opts)
     opts = opts or {}
     check_param_table(opts, { if_exists = 'boolean' })
+    check_schema_version()
     local _func = box.space[box.schema.FUNC_ID]
     local _vfunc = box.space[box.schema.VFUNC_ID]
     local fid
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 12078fc218..510ce27e26 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -667,6 +667,7 @@ end
 local function upgrade_priv_to_1_10_2()
     local _priv = box.space._priv
     local _vpriv = box.space._vpriv
+    local _index = box.space._index
     local format = _priv:format()
 
     format[4].type = 'scalar'
@@ -674,10 +675,15 @@ local function upgrade_priv_to_1_10_2()
     format = _vpriv:format()
     format[4].type = 'scalar'
     _vpriv:format(format)
-    _priv.index.primary:alter{parts={2, 'unsigned', 3, 'string', 4, 'scalar'}}
-    _vpriv.index.primary:alter{parts={2, 'unsigned', 3, 'string', 4, 'scalar'}}
-    _priv.index.object:alter{parts={3, 'string', 4, 'scalar'}}
-    _vpriv.index.object:alter{parts={3, 'string', 4, 'scalar'}}
+
+    _index:update({_priv.id, _priv.index.primary.id},
+                  {{'=', 'parts', {{1, 'unsigned'}, {2, 'string'}, {3, 'scalar'}}}})
+    _index:update({_vpriv.id, _vpriv.index.primary.id},
+                  {{'=', 'parts', {{1, 'unsigned'}, {2, 'string'}, {3, 'scalar'}}}})
+    _index:update({_priv.id, _priv.index.object.id},
+                  {{'=', 'parts', {{2, 'string'}, {3, 'scalar'}}}})
+    _index:update({_vpriv.id, _priv.index.object.id},
+                  {{'=', 'parts', {{2, 'string'}, {3, 'scalar'}}}})
 end
 
 local function create_vinyl_deferred_delete_space()
@@ -1079,8 +1085,9 @@ local function upgrade_func_to_2_2_1()
     format[18] = {name='created', type='string'}
     format[19] = {name='last_altered', type='string'}
     _func:format(format)
-    _func.index.name:alter({parts = {{'name', 'string',
-                                      collation = 'unicode_ci'}}})
+    box.space._index:update(
+        {_func.id, _func.index.name.id},
+        {{'=', 'parts', {{field = 2, type = 'string', collation = 2}}}})
 end
 
 local function create_func_index()
@@ -1146,7 +1153,8 @@ end
 
 local function drop_func_collation()
     local _func = box.space[box.schema.FUNC_ID]
-    _func.index.name:alter({parts = {{'name', 'string'}}})
+    box.space._index:update({_func.id, _func.index.name.id},
+                            {{'=', 'parts', {{2, 'string'}}}})
 end
 
 local function create_session_settings_space()
@@ -1306,7 +1314,11 @@ local function schema_needs_upgrade()
     local schema_version, schema_version_str = get_version()
     if schema_version ~= nil and
         handlers[#handlers].version > schema_version then
-        return true, schema_version_str
+        local msg = string.format(
+            'Your schema version is %s while Tarantool %s requires a more'..
+            ' recent schema version. Please, consider using box.'..
+            'schema.upgrade().', schema_version_str, box.info.version)
+        return true, msg
     end
     return false
 end
diff --git a/test/box-luatest/gh_7149_forbid_ddl_until_box_schema_upgrade_test.lua b/test/box-luatest/gh_7149_forbid_ddl_until_box_schema_upgrade_test.lua
index 6956ce6a92..b9bab98413 100644
--- a/test/box-luatest/gh_7149_forbid_ddl_until_box_schema_upgrade_test.lua
+++ b/test/box-luatest/gh_7149_forbid_ddl_until_box_schema_upgrade_test.lua
@@ -21,3 +21,62 @@ g.test_schema_access = function()
         box.schema.upgrade()
     end)
 end
+
+g.before_test('test_ddl_ops', function()
+    -- Recover from Tarantool 1.10 snapshot
+    local data_dir = 'test/box-luatest/upgrade/1.10'
+    -- Disable automatic schema upgrade
+    local box_cfg = {read_only = true}
+    g.server = server:new{alias = 'master',
+                          datadir = data_dir,
+                          box_cfg = box_cfg}
+    g.server:start()
+end)
+
+g.test_ddl_ops = function()
+    g.server:exec(function()
+        local t = require('luatest')
+        local error_msg = "DDL operations are not allowed: " ..
+                          "Your schema version is 1.10.0 while Tarantool "
+
+        -- Note that automatic schema upgrade will not be performed
+        box.cfg{read_only = false}
+
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.space.create('test') end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.space.alter(box.space.T1.id, {}) end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.space.drop(box.space.T1.id) end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.index.create(box.space.T1.id, 'name') end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.index.alter(box.space.T1.id, 0, {}) end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.index.drop(box.space.T1.id, 0) end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.sequence.create('test') end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.sequence.alter('test', {}) end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.sequence.drop('test') end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.func.create('test') end)
+        t.assert_error_msg_contains(error_msg,
+            function() box.schema.func.drop('test') end)
+
+        box.schema.upgrade()
+
+        box.schema.space.create('test')
+        box.schema.space.alter(box.space.test.id, {})
+        box.schema.space.drop(box.space.test.id)
+        box.schema.index.create(box.space.T1.id, 'name')
+        box.schema.index.alter(box.space.T1.id, 1, {})
+        box.schema.index.drop(box.space.T1.id, 1)
+        box.schema.sequence.create('test')
+        box.schema.sequence.alter('test', {})
+        box.schema.sequence.drop('test')
+        box.schema.func.create('test')
+        box.schema.func.drop('test')
+    end)
+end
diff --git a/test/box-luatest/upgrade/1.10/00000000000000000004.snap b/test/box-luatest/upgrade/1.10/00000000000000000004.snap
new file mode 100644
index 0000000000000000000000000000000000000000..61ed92fc61744ecf481b114ace6ed68eec6d0128
GIT binary patch
literal 1612
zcmV-S2DAB7PC-x#FfK7O3RY!ub7^mGIv_GGF)%JLEn;PKVKOZ=H!v+{VmV_uVK-th
zGYUy=b97;DV`VxZGBh$~WjSOxEn_e-VJ$RaF=8z^H8V0TG%_+cHf1zoW@2P!3RXjG
zZ)0mZAbT-7AT)gn3e~y`y3Ga00M0g-$5^ER00000D77#B09f@c0EU0yO3)ao0{~w<
zh$rv^@jybI7R+K1MF7^8`~pfk72Jd`ezgJFEtO-hbe&kT({A!5=`%={EBOEsMS)*d
zSYwyBkG5G=8NqdVDWwd`0Kfp(0G660o&oVrjyDm1qlhDGeZS5bPw;gtb6p03c?r*t
z9&^W{IDXzx9SL|KKtIF!7Snlmj(f7!Z#+Fhs7{~#?8LK|eca2{mRfYABL%pYnzfB*
zd6PL=kLll?2kqp~8?ry$2*9<}6q4k|i55Wh!STzw%+;T6bPZHJX1<@6Gjc68sVxGt
zr-Ns>dM!0uEg0)(WMDL*>q9C8KUgZHX|+<S*Ef?|xjq<3BSUk+%Kpwv@ypq|mP#p=
z7M`IiA{HXJDmGI9SjrRB{Md_`mNzNn_3wYcMxy_*YrJDQTHd60R#yX`{k&W;xR#o-
zUSgm9y!6hhYBBQZGNgI9idt|jHOv0o$fwKDJL^foPnTe0^2|U#IuGjX;VKLQBs6cR
znhsn`%~Hg))7CNQHK`N~TuV)jUmQo6_|;za;qUAlcYf+%oo7J%W8Zbok>B|ZiT*Ok
zb5E$c6v+8Z<29b6dAV95fcM5Hp6xNj#jqHjU6<qMf#T<oNavV-+>!q<$vs>p4Y&b5
zohrq()U>(r0;rCm&N2IW855&3!k*OX)16RIdm^afn82~Qsi`$3YDm0ByhOZ0*l^Yb
zg1TPUQuEr1xLhlm)rZxLs-==qu}~|Nst4ua6mv>lOU<5BhmM1-+W>>I!JJGQOc_iV
zWE{&VTyU}AQo*Gw!IWM~CTmF)s;wtVCAp9$)H;%IEj33aYU`2mWss~(+|K{*H`-E*
zemWS^wbay6(zC)@QmKsqe#iUu$2^{y5EXG!aUfhvO<TvGE_e5Y>dk{|sae{*pjtDc
z<xRdETpQE|z0{gM?h7?5-RW`b023V&g0K+Q`VTG^tCeEO%(zmp6>g>QO7*2&;Oj$H
zi*><DwRJT-UUulHxjF0&j@OM94$4MX!$hN&W?V~6JE}#el~MJW`=`q=$i}#qnpv`B
z$wH;VxR#nE`g}hv>oVxYi;HWi3Bc0o)46W;lO;dvON%dy<Zq&SS-d#z+m}UHN#d&Y
z=~OFpo`LwRRmHW`91#m;WQ~h9E>wIRXz=))T8yqDUF1AXMuW1#GM*x_>=WAA&&ylq
zU&hW#1A!{2(OhIqjm(H71qDf^0|5p{A?J)95`e%k2&6a)VHgHP2r!CH7!i@cBqSk{
zR0{_M28FCK7F|=&k1ffbD}R!AY76A}c`xtfy}XxK0y#TiM8~Vmx|a*ylNkV6vq}CZ
zDH&r!dW{*DlT4?2i-@56jpT|yY??R12-MJ>svlZ?T&p-O0O2%074GmbwQBLMahNcs
zMLnAm(cvt>V_X;5t+wjQEF>WIHQR7CeHX}oqNfLDy&mtZ#&P6VEu*oC+PRSl7LtOB
z2o{opiU?S?EH%9XmF_HP$URXE>%XlbgM@oF5kDt!NrHo5ZXlf*jSZ(IzMKcSY^Vt=
zl7x9RM}gZ~HCKt+?WBTb8@IhZF{^9vvN`kuTqMyqBl|RP@t?HGv|SSj8P&v!oS`g0
z0VJ9=0O)RlV~OrkWF%<42e27^Zfq692xY3X4!sW)DY-^LV&v;-P;aa@cN&eMmbeEX
z?2~twBZq}x&q`d|%kY(~5LAkgO#zYatxM+(GGX>J9Qm)Vi`ZQq<(b=$!Z;`!YK+30
z4Hdl&E*8_(ZZO7;No5P9|BkcNwjOe3de8BhFrc2!+^K;OLvCyRdZKYS4V@JUKnlSZ
z5AxG(0ThLyq)ISwJ*(zq3fK-|(CK^WjNTLFmVv3@l|joi=4R+4uvtVi(m#?!^s8=0
zf-|LE9tFZNSsv_Y?ow8zV6fW42cBu2iMnoIWD?f3)wuY7L&@2=Wj_aAj%l-hu7aXT
Ka%`&A5UuT=Liwct

literal 0
HcmV?d00001

diff --git a/test/box/error.result b/test/box/error.result
index a0633dbdae..ba7bc0b5b8 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -466,6 +466,7 @@ t;
  |   245: box.error.OLD_TERM
  |   246: box.error.INTERFERING_ELECTIONS
  |   247: box.error.ITERATOR_POSITION
+ |   248: box.error.DDL_NOT_ALLOWED
  | ...
 
 test_run:cmd("setopt delimiter ''");
-- 
GitLab