Skip to content

Commit

Permalink
fix(query): privilege type forward compat (#14501)
Browse files Browse the repository at this point in the history
* fix(query): privilege type forward compat

1. add ci test forward compat test(now only test rbac)
2. UserPrivilegeType and ShareGrantObjectPrivilege deserialize use from_bits_truncate

* run_test contain forward and backward
  • Loading branch information
TCeason committed Jan 29, 2024
1 parent 5a76396 commit 0c05f06
Show file tree
Hide file tree
Showing 11 changed files with 215 additions and 54 deletions.
2 changes: 2 additions & 0 deletions .github/actions/fuse_compat/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ runs:
bash ./tests/fuse-compat/test-fuse-compat.sh 1.1.39 base
bash ./tests/fuse-compat/test-fuse-compat.sh 1.1.46 base
bash ./tests/fuse-compat/test-fuse-compat.sh 1.2.306 rbac
bash ./tests/fuse-compat/test-fuse-forward-compat.sh 1.2.307 rbac
bash ./tests/fuse-compat/test-fuse-forward-compat.sh 1.2.318 rbac
- name: Upload failure
if: failure()
uses: ./.github/actions/artifact_failure
Expand Down
1 change: 1 addition & 0 deletions src/meta/app/src/principal/user_privilege.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use enumflags2::bitflags;
use enumflags2::make_bitflags;
use enumflags2::BitFlags;

// Note: If add new privilege type, need add forward test
#[bitflags]
#[repr(u64)]
#[derive(
Expand Down
30 changes: 13 additions & 17 deletions src/meta/proto-conv/src/share_from_to_protobuf_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,23 +129,19 @@ impl FromToProto for mt::ShareGrantEntry {
where Self: Sized {
reader_check_msg(p.ver, p.min_reader_ver)?;

let privileges = BitFlags::<mt::ShareGrantObjectPrivilege, u64>::from_bits(p.privileges);
match privileges {
Ok(privileges) => Ok(mt::ShareGrantEntry {
object: mt::ShareGrantObject::from_pb(p.object.ok_or_else(|| Incompatible {
reason: "ShareGrantEntry.object can not be None".to_string(),
})?)?,
privileges,
grant_on: DateTime::<Utc>::from_pb(p.grant_on)?,
update_on: match p.update_on {
Some(t) => Some(DateTime::<Utc>::from_pb(t)?),
None => None,
},
}),
Err(e) => Err(Incompatible {
reason: format!("UserPrivilegeType error: {}", e),
}),
}
let privileges =
BitFlags::<mt::ShareGrantObjectPrivilege, u64>::from_bits_truncate(p.privileges);
Ok(mt::ShareGrantEntry {
object: mt::ShareGrantObject::from_pb(p.object.ok_or_else(|| Incompatible {
reason: "ShareGrantEntry.object can not be None".to_string(),
})?)?,
privileges,
grant_on: DateTime::<Utc>::from_pb(p.grant_on)?,
update_on: match p.update_on {
Some(t) => Some(DateTime::<Utc>::from_pb(t)?),
None => None,
},
})
}

fn to_pb(&self) -> Result<pb::ShareGrantEntry, Incompatible> {
Expand Down
20 changes: 8 additions & 12 deletions src/meta/proto-conv/src/user_from_to_protobuf_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,18 +241,14 @@ impl FromToProto for mt::principal::GrantEntry {
where Self: Sized {
reader_check_msg(p.ver, p.min_reader_ver)?;

let privileges = BitFlags::<mt::principal::UserPrivilegeType, u64>::from_bits(p.privileges);
match privileges {
Ok(privileges) => Ok(mt::principal::GrantEntry::new(
mt::principal::GrantObject::from_pb(p.object.ok_or_else(|| Incompatible {
reason: "GrantEntry.object can not be None".to_string(),
})?)?,
privileges,
)),
Err(e) => Err(Incompatible {
reason: format!("UserPrivilegeType error: {}", e),
}),
}
let privileges =
BitFlags::<mt::principal::UserPrivilegeType, u64>::from_bits_truncate(p.privileges);
Ok(mt::principal::GrantEntry::new(
mt::principal::GrantObject::from_pb(p.object.ok_or_else(|| Incompatible {
reason: "GrantEntry.object can not be None".to_string(),
})?)?,
privileges,
))
}

fn to_pb(&self) -> Result<pb::GrantEntry, Incompatible> {
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ const META_CHANGE_LOG: &[(u64, &str)] = &[
(75, "2024-01-15: ADD: user.proto/CsvFileFormatParams add field `binary_format` and `output_header`", ),
(76, "2024-01-18: ADD: ownership.proto and role.proto", ),
(77, "2024-01-22: Remove: allow_anonymous in S3 Config", ),
(78, "2024-01-29: Refactor: GrantEntry::UserPrivilegeType and ShareGrantEntry::ShareGrantObjectPrivilege use from_bits_truncate deserialize", ),
// Dear developer:
// If you're gonna add a new metadata version, you'll have to add a test for it.
// You could just copy an existing test file(e.g., `../tests/it/v024_table_meta.rs`)
Expand Down
1 change: 1 addition & 0 deletions src/meta/proto-conv/tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,4 @@ mod v074_table_db_meta;
mod v075_csv_format_params;
mod v076_role_ownership_info;
mod v077_s3_remove_allow_anonymous;
mod v078_grantentry;
72 changes: 72 additions & 0 deletions src/meta/proto-conv/tests/it/v078_grantentry.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2023 Datafuse Labs.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use chrono::TimeZone;
use chrono::Utc;
use databend_common_meta_app as mt;
use databend_common_meta_app::principal::UserPrivilegeType;
use databend_common_meta_app::share::ShareGrantObjectPrivilege;
use enumflags2::make_bitflags;
use minitrace::func_name;

use crate::common;

// These bytes are built when a new version in introduced,
// and are kept for backward compatibility test.
//
// *************************************************************
// * These messages should never be updated, *
// * only be added when a new version is added, *
// * or be removed when an old version is no longer supported. *
// *************************************************************
//

#[test]
fn test_decode_v78_grant_entry() -> anyhow::Result<()> {
let grant_entry_v78 = vec![
10, 8, 10, 0, 160, 6, 78, 168, 6, 24, 16, 254, 255, 55, 160, 6, 78, 168, 6, 24,
];

let want = || {
mt::principal::GrantEntry::new(
mt::principal::GrantObject::Global,
make_bitflags!(UserPrivilegeType::{Create | Select | Insert | Update | Delete | Drop | Alter | Super | CreateUser | DropUser | CreateRole | DropRole | Grant | CreateStage | Set | CreateDataMask | Read | Write }),
)
};

common::test_pb_from_to(func_name!(), want())?;
common::test_load_old(func_name!(), grant_entry_v78.as_slice(), 78, want())?;

Ok(())
}

#[test]
fn test_decode_v78_share_grant_entry() -> anyhow::Result<()> {
let share_grant_entry_v78 = vec![
10, 8, 16, 19, 160, 6, 78, 168, 6, 24, 16, 4, 26, 23, 50, 48, 49, 52, 45, 49, 49, 45, 50,
56, 32, 49, 50, 58, 48, 48, 58, 48, 57, 32, 85, 84, 67, 160, 6, 78, 168, 6, 24,
];

let want = || mt::share::ShareGrantEntry {
object: mt::share::ShareGrantObject::Table(19),
privileges: make_bitflags!(ShareGrantObjectPrivilege::{Select}),
grant_on: Utc.with_ymd_and_hms(2014, 11, 28, 12, 0, 9).unwrap(),
update_on: None,
};

common::test_pb_from_to(func_name!(), want())?;
common::test_load_old(func_name!(), share_grant_entry_v78.as_slice(), 78, want())?;

Ok(())
}
4 changes: 4 additions & 0 deletions tests/fuse-compat/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ by a older version databend-query.

```shell
tests/fuse-compat/test-fuse-compat.sh <old_ver>
tests/fuse-compat/test-fuse-forward-compat.sh <old_ver>
```

E.g. `tests/fuse-compat/test-fuse-compat.sh 0.7.151` tests if the fuse-table written
by **databend-query-0.7.151** can be read by **current** version databend-query.

`tests/fuse-compat/test-fuse-forward-compat.sh 1.2.307` tests if the fuse-table written
by **current** can be read by **databend-query-0.7.151** version databend-query.

## Prerequisites

- Current version of databend-query and databend-meta must reside in `./bins`:
Expand Down
2 changes: 1 addition & 1 deletion tests/fuse-compat/test-fuse-compat.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ download_query_config "$old_query_ver" old_config
download_binary "$old_query_ver"

old_config_path="old_config/$query_config_path"
run_test $old_query_ver $old_config_path $logictest_path
run_test $old_query_ver $old_config_path $logictest_path "backward"

if [ -n "$stateless_test_path" ];
then
Expand Down
64 changes: 64 additions & 0 deletions tests/fuse-compat/test-fuse-forward-compat.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#!/bin/bash

set -o errexit
SCRIPT_PATH="$(cd "$(dirname "$0")" >/dev/null 2>&1 && pwd)"
echo " === SCRIPT_PATH: $SCRIPT_PATH"
# go to work tree root
cd "$SCRIPT_PATH/../../"
ROOT="$(pwd)"
pwd

export RUST_BACKTRACE=full

BUILD_PROFILE="${BUILD_PROFILE:-debug}"

query_config_path="scripts/ci/deploy/config/databend-query-node-1.toml"

usage() {
echo " === Assert that an old version query being compatible with lastest version query on fuse-table format"
echo " === Expect ./bins/current contains current version binaries"
echo " === Usage: $0 <old_version> <logictest_path> <supplementray_statless_test_path>"
}

source "${SCRIPT_PATH}/util.sh"


# -- main --

# The previous version to assert compatibility with
# e.g. old_query_ver="0.7.151"
old_query_ver="$1"

# default sqllogic test suite is "testlogictest_path=${2:-"./base"}s/fuse-forward-compat/compat-logictest/"
logictest_path=${2:-"./base"}

# supplementary stateless test suite if provided (optional), which will be searched under "tests/fuse-forward-compat/compat-stateless"
stateless_test_path="$3"

echo " === old query ver : ${old_query_ver}"
echo " === sql logic test path: ${logictest_path}"
echo " === supplementary stateless test path: ${stateless_test_path}"


chmod +x ./bins/current/*

echo " === current metasrv ver: $(./bins/current/databend-meta --single --cmd ver | tr '\n' ' ')"
echo " === current query ver: $(./bins/current/databend-query --cmd ver | tr '\n' ' ')"
echo " === old query ver: $old_query_ver"


mkdir -p ./target/${BUILD_PROFILE}/

download_query_config "$old_query_ver" old_config
download_binary "$old_query_ver"

echo "=== Now test forward compat ==="

old_config_path="old_config/$query_config_path"
run_test $old_query_ver $old_config_path $logictest_path "forward"

if [ -n "$stateless_test_path" ];
then
echo "=== ruing supplementary stateless test: ${stateless_test_path}"
run_stateless $stateless_test_path
fi
72 changes: 48 additions & 24 deletions tests/fuse-compat/util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ run_test() {
local query_old_ver="$1"
local old_config_path="$2"
local logictest_path="tests/fuse-compat/compat-logictest/$3"
local forward="$4"

echo " === Test with query-$query_old_ver and current query"

Expand Down Expand Up @@ -150,47 +151,70 @@ run_test() {

export RUST_BACKTRACE=1

echo ' === Start old databend-meta...'
if [ "$forward" == "forward" ]
then
echo ' === Start new databend-meta and databend-query...'
config_path="scripts/ci/deploy/config/databend-query-node-1.toml"
log="query-current.log"
start $metasrv_new $query_new $config_path $log

nohup "$metasrv_old" --single --log-level=DEBUG &
python3 scripts/ci/wait_tcp.py --timeout 10 --port 9191

echo ' === Start old databend-query...'

# TODO clean up data?
echo " === bring up $query_old"
echo " === Run test: fuse_compat_write with current query"
else
echo ' === Start old databend-meta...'

nohup "$query_old" -c "$old_config_path" --log-level DEBUG --meta-endpoints "0.0.0.0:9191" >query-old.log &
python3 scripts/ci/wait_tcp.py --timeout 15 --port 3307
echo ' === Start old databend-meta and databend-query...'
log="query-old.log"
start "$metasrv_old" "$query_old" "$old_config_path" $log

echo " === Run test: fuse_compat_write with old query"
echo " === Run test: fuse_compat_write with old query"
fi

# download_logictest $query_old_ver old_logictest/$query_old_ver
# run_logictest old_logictest/$query_old_ver fuse_compat_write
# if backward
# run_logictest new_logictest/$query_old_ver fuse_compat_write
# if forward
# run_logictest new_logictest/$query_new_ver fuse_compat_write
$sqllogictests --handlers mysql --suites "$logictest_path" --run_file fuse_compat_write

kill_proc databend-query
kill_proc databend-meta

echo ' === Start new databend-meta...'
if [ "$forward" == 'forward' ]
then
echo " === Start old databend-meta and databend-query..."
log="query-old.log"
start "$metasrv_old" "$query_old" "$old_config_path" $log
echo " === Run test: fuse_compat_read with old query"
else
echo ' === Start new databend-meta and databend-query...'
config_path="scripts/ci/deploy/config/databend-query-node-1.toml"
log="query-current.log"
start $metasrv_new $query_new $config_path $log
echo " === Run test: fuse_compat_read with current query"
fi

nohup "$metasrv_new" --single --log-level=DEBUG &
python3 scripts/ci/wait_tcp.py --timeout 20 --port 9191
# download_logictest $query_old_ver old_logictest
$sqllogictests --handlers mysql --suites "$logictest_path" --run_file fuse_compat_read
}

echo " === Start new databend-query..."
start() {
local metasrv="$1"
local query="$2"
local config_path="$3"
local log="$4"
export RUST_BACKTRACE=1
echo " === Start $metasrv databend-meta..."

config_path="scripts/ci/deploy/config/databend-query-node-1.toml"
echo "new databend config path: $config_path"
nohup "$metasrv" --single --log-level=DEBUG &
python3 scripts/ci/wait_tcp.py --timeout 20 --port 9191

nohup "$query_new" -c "$config_path" --log-level DEBUG --meta-endpoints "0.0.0.0:9191" >query-current.log &
python3 scripts/ci/wait_tcp.py --timeout 30 --port 3307
echo " === Start $query databend-query..."

echo " === Run test: fuse_compat_read with current query"
echo "databend config path: $config_path"

# download_logictest $query_old_ver old_logictest
$sqllogictests --handlers mysql --suites "$logictest_path" --run_file fuse_compat_read
nohup "$query" -c "$config_path" --log-level DEBUG --meta-endpoints "0.0.0.0:9191" > "$log" &
python3 scripts/ci/wait_tcp.py --timeout 30 --port 3307
}

# Run suppelmentary stateless tests
run_stateless() {
local case_path="$1"
Expand Down

0 comments on commit 0c05f06

Please sign in to comment.