Allow to use memtable_flush_period_in_ms schema option for system tables
It's possible to modify 'memtable_flush_period_in_ms' option only and as single option, not with any other options together Refs #20999 Fixes #21223 Closes scylladb/scylladb#22536
This commit is contained in:
committed by
Botond Dénes
parent
d50738feca
commit
cc35905531
@@ -676,7 +676,8 @@ future<permission_set> get_permissions(const service& ser, const authenticated_u
|
|||||||
}
|
}
|
||||||
|
|
||||||
bool is_protected(const service& ser, command_desc cmd) noexcept {
|
bool is_protected(const service& ser, command_desc cmd) noexcept {
|
||||||
if (cmd.type_ == command_desc::type::ALTER_WITH_OPTS) {
|
if (cmd.type_ == command_desc::type::ALTER_WITH_OPTS ||
|
||||||
|
cmd.type_ == command_desc::type::ALTER_SYSTEM_WITH_ALLOWED_OPTS) {
|
||||||
return false; // Table attributes are OK to modify; see #7057.
|
return false; // Table attributes are OK to modify; see #7057.
|
||||||
}
|
}
|
||||||
return ser.underlying_role_manager().protected_resources().contains(cmd.resource)
|
return ser.underlying_role_manager().protected_resources().contains(cmd.resource)
|
||||||
|
|||||||
@@ -224,6 +224,7 @@ struct command_desc {
|
|||||||
const ::auth::resource& resource; ///< Resource impacted by this command.
|
const ::auth::resource& resource; ///< Resource impacted by this command.
|
||||||
enum class type {
|
enum class type {
|
||||||
ALTER_WITH_OPTS, ///< Command is ALTER ... WITH ...
|
ALTER_WITH_OPTS, ///< Command is ALTER ... WITH ...
|
||||||
|
ALTER_SYSTEM_WITH_ALLOWED_OPTS,
|
||||||
OTHER
|
OTHER
|
||||||
} type_ = type::OTHER;
|
} type_ = type::OTHER;
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -57,8 +57,16 @@ uint32_t alter_table_statement::get_bound_terms() const {
|
|||||||
|
|
||||||
future<> alter_table_statement::check_access(query_processor& qp, const service::client_state& state) const {
|
future<> alter_table_statement::check_access(query_processor& qp, const service::client_state& state) const {
|
||||||
using cdt = auth::command_desc::type;
|
using cdt = auth::command_desc::type;
|
||||||
return state.has_column_family_access(keyspace(), column_family(), auth::permission::ALTER,
|
auto type = cdt::OTHER;
|
||||||
_type == type::opts ? cdt::ALTER_WITH_OPTS : cdt::OTHER);
|
if (_type == type::opts) {
|
||||||
|
// can modify only KW_MEMTABLE_FLUSH_PERIOD property for system tables (see issue #21223)
|
||||||
|
if (is_system_keyspace(keyspace()) && _properties->count() == 1 && _properties->has_property(cf_prop_defs::KW_MEMTABLE_FLUSH_PERIOD)) {
|
||||||
|
type = cdt::ALTER_SYSTEM_WITH_ALLOWED_OPTS;
|
||||||
|
} else {
|
||||||
|
type = cdt::ALTER_WITH_OPTS;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return state.has_column_family_access(keyspace(), column_family(), auth::permission::ALTER, type);
|
||||||
}
|
}
|
||||||
|
|
||||||
static data_type validate_alter(const schema& schema, const column_definition& def, const cql3_type& validator)
|
static data_type validate_alter(const schema& schema, const column_definition& def, const cql3_type& validator)
|
||||||
|
|||||||
@@ -70,6 +70,10 @@ public:
|
|||||||
static int32_t to_int(sstring key, std::optional<sstring> value, int32_t default_value);
|
static int32_t to_int(sstring key, std::optional<sstring> value, int32_t default_value);
|
||||||
|
|
||||||
static long to_long(sstring key, std::optional<sstring> value, long default_value);
|
static long to_long(sstring key, std::optional<sstring> value, long default_value);
|
||||||
|
|
||||||
|
size_t count() const {
|
||||||
|
return _properties.size();
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ future<> service::client_state::has_access(const sstring& ks, auth::command_desc
|
|||||||
// prevent system keyspace modification
|
// prevent system keyspace modification
|
||||||
auto name = ks;
|
auto name = ks;
|
||||||
std::transform(name.begin(), name.end(), name.begin(), ::tolower);
|
std::transform(name.begin(), name.end(), name.begin(), ::tolower);
|
||||||
if (is_system_keyspace(name)) {
|
if (is_system_keyspace(name) && cmd.type_ != auth::command_desc::type::ALTER_SYSTEM_WITH_ALLOWED_OPTS) {
|
||||||
return make_exception_future<>(exceptions::unauthorized_exception(ks + " keyspace is not user-modifiable."));
|
return make_exception_future<>(exceptions::unauthorized_exception(ks + " keyspace is not user-modifiable."));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -11,6 +11,7 @@
|
|||||||
from .util import new_test_table
|
from .util import new_test_table
|
||||||
import pytest
|
import pytest
|
||||||
from . import nodetool
|
from . import nodetool
|
||||||
|
from cassandra.protocol import Unauthorized
|
||||||
|
|
||||||
#############################################################################
|
#############################################################################
|
||||||
# system.size_estimates.partitions_count
|
# system.size_estimates.partitions_count
|
||||||
@@ -133,3 +134,15 @@ def test_partitions_estimate_only_deletions(cassandra_bug, cql, test_keyspace):
|
|||||||
print(count)
|
print(count)
|
||||||
# Count should be close to 0, not to N
|
# Count should be close to 0, not to N
|
||||||
assert count < N/1.25
|
assert count < N/1.25
|
||||||
|
|
||||||
|
# See issue #21223
|
||||||
|
# Test possibility to set 'memtable_flush_period_in_ms' option for system tables
|
||||||
|
# and this option only: it is impossible to modify it with any other options together
|
||||||
|
def test_alter_system_table_properties(cql, test_keyspace):
|
||||||
|
with pytest.raises(Unauthorized):
|
||||||
|
cql.execute("ALTER TABLE system.compaction_history WITH comment = ''")
|
||||||
|
|
||||||
|
with pytest.raises(Unauthorized):
|
||||||
|
cql.execute("ALTER TABLE system.compaction_history WITH memtable_flush_period_in_ms = 80000 AND comment = ''")
|
||||||
|
|
||||||
|
cql.execute("ALTER TABLE system.compaction_history WITH memtable_flush_period_in_ms = 80000")
|
||||||
|
|||||||
Reference in New Issue
Block a user