[TNTP-3536] validate bucket_id in CRUD operations

(cherry picked from commit 3ef295df0e726568905fd5f67deff5feb50f406e)

Author: Александр Вахов

CHANGELOG.md

@@ -6,9 +6,19 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-* Обеспечена поддержка ролевой модели в CRUD (https://jira.vk.team/browse/TNTP-2177)
+
+### Changed
+* bumped: metrics version in rockspec
+* bumped: cartridge version in rockspec
+* bumped: vshard version in rockspec
+* Role model support in CRUD is provided (https://jira.vk.team/browse/TNTP-2177)
+
+### Added
+* Validation of `bucket_id`. Invalid values now raise `BucketIDError`
+  before routing. [TNTP-3536](https://jira.vk.team/browse/TNTP-3536)
 
 ### Fixed
+* Fixed compatibility with cartridge `2.16.0` ([TNTP-3598](https://jira.vk.team/browse/TNTP-3598))
 * `crud.schema` no longer returns TCF system space `_cdc_state`.
 * `crud.schema` no longer returns system space `_gc_consumers` with Tarantool 3.2+.
 * `crud.schema` no longer returns `tt` system space `_tt_migrations`.

Makefile

@@ -1,20 +1,18 @@
 SHELL := /bin/bash
 
-S3_TARANTOOL_SDK_3_PATH := s3://packages/enterprise/release/linux/x86_64/3.3/tarantool-enterprise-sdk-gc64-3.3.1-0-r55.linux.x86_64.tar.gz
-S3_TARANTOOL_SDK_2_PATH := s3://packages/enterprise/release/linux/x86_64/2.11/tarantool-enterprise-sdk-gc64-2.11.7-0-r689.linux.x86_64.tar.gz
+S3_TARANTOOL_SDK_3_PATH := s3://packages/enterprise/release/linux/x86_64/3.3/tarantool-enterprise-sdk-gc64-3.3.2-0-r62.linux.x86_64.tar.gz
+S3_TARANTOOL_SDK_2_PATH := s3://packages/enterprise/release/linux/x86_64/2.11/tarantool-enterprise-sdk-gc64-2.11.7-0-r691.linux.x86_64.tar.gz
 S3_ENDPOINT_URL := $(if $(S3_ENDPOINT_URL),$(S3_ENDPOINT_URL),https://hb.vkcs.cloud)
 
-SDK_TEST := $(if $(SDK_TEST),$(SDK_TEST),sdk-3)
-
 .rocks: sdk
 	# в sdk-2 есть все нужные роки, в sdk-3 нет
 	source ./sdk-2/env.sh && \
 	tt rocks install luacheck 0.26.0 --only-server=sdk-2/rocks && \
 	tt rocks install luacov 0.13.0 --only-server=sdk-2/rocks && \
 	tt rocks install luacov-reporters 0.1.0 --only-server=sdk-2/rocks && \
-	tt rocks install metrics  1.1.0 --only-server=sdk-2/rocks && \
+	tt rocks install metrics  1.4.0 --only-server=sdk-2/rocks && \
 	tt rocks install ddl-ee 1.8.0 --only-server=sdk-2/rocks && \
-	tt rocks install cartridge 2.16.0 --only-server=sdk-2/rocks && \
+	tt rocks install cartridge 2.16.2 --only-server=sdk-2/rocks && \
 	tt rocks install migrations-ee 1.3.2 --only-server=sdk-2/rocks && \
 	tt rocks make
 
@@ -38,10 +36,28 @@ lint: .rocks
 
 .PHONY: test
 test:
-	@echo "RUN TESTS WITH $(SDK_TEST)"
-	# luatest будет свой для каждого sdk
-	source $(SDK_TEST)/env.sh && \
-	tt rocks install luatest 1.0.1 --only-server=$(SDK_TEST)/rocks && \
+	@if [ -z "$(SDK_TEST)" ]; then \
+		echo "Select SDK:"; \
+		echo "1) SDK with Tarantool 2.x"; \
+		echo "2) SDK with Tarantool 3.x"; \
+		read -p "Enter number (1 or 2): " choice; \
+		case $$choice in \
+			1) SDK_TEST=sdk-2; SDK_LABEL="SDK with Tarantool 2.x" ;; \
+			2) SDK_TEST=sdk-3; SDK_LABEL="SDK with Tarantool 3.x" ;; \
+			*) echo "Invalid selection" >&2; exit 1 ;; \
+		esac; \
+	else \
+		if [ "$(SDK_TEST)" = "sdk-2" ]; then \
+			SDK_LABEL="SDK with Tarantool 2.x"; \
+		elif [ "$(SDK_TEST)" = "sdk-3" ]; then \
+			SDK_LABEL="SDK with Tarantool 3.x"; \
+		else \
+			SDK_LABEL="Custom SDK ($(SDK_TEST))"; \
+		fi; \
+	fi; \
+	echo "Running tests with $$SDK_LABEL..."; \
+	source $$SDK_TEST/env.sh && \
+	tt rocks install luatest 1.0.1 --only-server=$$SDK_TEST/rocks && \
 	.rocks/bin/luatest -v --coverage test/
 
 coverage:

crud/common/sharding/init.lua

@@ -29,9 +29,14 @@ function sharding.get_replicasets_by_bucket_id(vshard_router, bucket_id)
 end
 
 function sharding.key_get_bucket_id(vshard_router, space_name, key, specified_bucket_id)
-    dev_checks('table', 'string', '?', '?number|cdata')
+    dev_checks('table', 'string', '?', '?')
 
     if specified_bucket_id ~= nil then
+        local err = sharding.validate_bucket_id(specified_bucket_id)
+        if err ~= nil then
+            return nil, err
+        end
+
         return { bucket_id = specified_bucket_id }
     end
 
@@ -77,6 +82,15 @@ function sharding.tuple_get_bucket_id(vshard_router, tuple, space, specified_buc
     }
 end
 
+function sharding.validate_bucket_id(bucket_id)
+    if not utils.is_uint(bucket_id) or bucket_id < 1 then
+        return BucketIDError:new(
+                "Invalid bucket_id: expected unsigned, got %s",
+                type(bucket_id)
+        )
+    end
+end
+
 function sharding.tuple_set_and_return_bucket_id(vshard_router, tuple, space, specified_bucket_id)
     local bucket_id_fieldno, err = utils.get_bucket_id_fieldno(space)
     if err ~= nil then
@@ -108,6 +122,11 @@ function sharding.tuple_set_and_return_bucket_id(vshard_router, tuple, space, sp
         sharding_data.skip_sharding_hash_check = true
     end
 
+    err = sharding.validate_bucket_id(sharding_data.bucket_id)
+    if err ~= nil then
+        return nil, err
+    end
+
     return sharding_data
 end
 

crud/common/utils.lua

@@ -1338,4 +1338,15 @@ function utils.append_array(array_src, array_dst)
     return array_src
 end
 
+function utils.is_uint(value)
+    if type(value) == 'number' then
+        return value >= 0 and math.floor(value) == value
+    elseif type(value) == 'cdata' then
+        local ok, casted = pcall(tonumber, value)
+        return ok and type(casted) == 'number' and casted >= 0 and math.floor(casted) == casted
+    end
+
+    return false
+end
+
 return utils

crud/compare/plan.lua

@@ -168,7 +168,7 @@ function plan.new(space, conditions, opts)
         field_names = '?table',
         force_map_call = '?boolean',
         sharding_key_as_index_obj = '?table',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
     })
 
     conditions = conditions ~= nil and conditions or {}

crud/count.lua

@@ -110,7 +110,7 @@ end
 local function call_count_on_router(vshard_router, space_name, user_conditions, opts)
     checks('table', 'string', '?table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         force_map_call = '?boolean',
         fullscan = '?boolean',
         yield_every = '?number',
@@ -317,7 +317,7 @@ end
 function count.call(space_name, user_conditions, opts)
     checks('string', '?table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         force_map_call = '?boolean',
         fullscan = '?boolean',
         yield_every = '?number',

crud/delete.lua

@@ -60,7 +60,7 @@ delete.storage_api = {[DELETE_FUNC_NAME] = delete_on_storage}
 local function call_delete_on_router(vshard_router, space_name, key, opts)
     dev_checks('table', 'string', '?', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         fields = '?table',
         vshard_router = '?string|table',
         noreturn = '?boolean',
@@ -194,7 +194,7 @@ end
 function delete.call(space_name, key, opts)
     checks('string', '?', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         fields = '?table',
         vshard_router = '?string|table',
         noreturn = '?boolean',

crud/get.lua

@@ -58,7 +58,7 @@ get.storage_api = {[GET_FUNC_NAME] = get_on_storage}
 local function call_get_on_router(vshard_router, space_name, key, opts)
     dev_checks('table', 'string', '?', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         fields = '?table',
         prefer_replica = '?boolean',
         balance = '?boolean',
@@ -196,7 +196,7 @@ end
 function get.call(space_name, key, opts)
     checks('string', '?', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         fields = '?table',
         prefer_replica = '?boolean',
         balance = '?boolean',

crud/insert.lua

@@ -61,7 +61,7 @@ insert.storage_api = {[INSERT_FUNC_NAME] = insert_on_storage}
 local function call_insert_on_router(vshard_router, space_name, original_tuple, opts)
     dev_checks('table', 'string', 'table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         add_space_schema_hash = '?boolean',
         fields = '?table',
         vshard_router = '?string|table',
@@ -175,7 +175,7 @@ end
 function insert.tuple(space_name, tuple, opts)
     checks('string', 'table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         add_space_schema_hash = '?boolean',
         fields = '?table',
         vshard_router = '?string|table',
@@ -214,7 +214,7 @@ end
 function insert.object(space_name, obj, opts)
     checks('string', 'table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         add_space_schema_hash = '?boolean',
         fields = '?table',
         vshard_router = '?string|table',

crud/replace.lua

@@ -61,7 +61,7 @@ replace.storage_api = {[REPLACE_FUNC_NAME] = replace_on_storage}
 local function call_replace_on_router(vshard_router, space_name, original_tuple, opts)
     dev_checks('table', 'string', 'table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         add_space_schema_hash = '?boolean',
         fields = '?table',
         vshard_router = '?string|table',
@@ -174,7 +174,7 @@ end
 function replace.tuple(space_name, tuple, opts)
     checks('string', 'table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         add_space_schema_hash = '?boolean',
         fields = '?table',
         vshard_router = '?string|table',
@@ -213,7 +213,7 @@ end
 function replace.object(space_name, obj, opts)
     checks('string', 'table', {
         timeout = '?number',
-        bucket_id = '?number|cdata',
+        bucket_id = '?',
         add_space_schema_hash = '?boolean',
         fields = '?table',
         vshard_router = '?string|table',