diff --git a/auth/authentication_options.hh b/auth/authentication_options.hh index 4d613f985a..1ee5a07f4c 100644 --- a/auth/authentication_options.hh +++ b/auth/authentication_options.hh @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -22,6 +23,7 @@ namespace auth { enum class authentication_option { password, + salted_hash, options }; @@ -35,6 +37,8 @@ struct fmt::formatter : fmt::formatter switch (a) { case password: return formatter::format("PASSWORD", ctx); + case salted_hash: + return formatter::format("SALTED HASH", ctx); case options: return formatter::format("OPTIONS", ctx); } @@ -48,13 +52,22 @@ using authentication_option_set = std::unordered_set; using custom_options = std::unordered_map; +struct password_option { + sstring password; +}; + +/// Used exclusively for restoring roles. +struct salted_hash_option { + sstring salted_hash; +}; + struct authentication_options final { - std::optional password; + std::optional> credentials; std::optional options; }; inline bool any_authentication_options(const authentication_options& aos) noexcept { - return aos.password || aos.options; + return aos.options || aos.credentials; } class unsupported_authentication_option : public std::invalid_argument { diff --git a/auth/authenticator.hh b/auth/authenticator.hh index 6663b3f0ce..45de9a3d40 100644 --- a/auth/authenticator.hh +++ b/auth/authenticator.hh @@ -43,6 +43,11 @@ struct certificate_info { using session_dn_func = std::function>()>; +class unsupported_authentication_operation : public std::invalid_argument { +public: + using std::invalid_argument::invalid_argument; +}; + /// /// Abstract client for authenticating role identity. /// @@ -129,6 +134,19 @@ public: /// virtual future query_custom_options(std::string_view role_name) const = 0; + virtual bool uses_password_hashes() const { + return false; + } + + /// + /// Query the password hash corresponding to a given role. + /// + /// If the authenticator doesn't use password hashes, throws an `unsupported_authentication_operation` exception. + /// + virtual future> get_password_hash(std::string_view role_name) const { + return make_exception_future>(unsupported_authentication_operation("get_password_hash is not implemented")); + } + /// /// System resources used internally as part of the implementation. These are made inaccessible to users. /// diff --git a/auth/maintenance_socket_role_manager.cc b/auth/maintenance_socket_role_manager.cc index 3bd250f6f7..925f3f9d05 100644 --- a/auth/maintenance_socket_role_manager.cc +++ b/auth/maintenance_socket_role_manager.cc @@ -11,6 +11,7 @@ #include #include #include +#include "cql3/description.hh" #include "utils/class_registrator.hh" namespace auth { @@ -109,4 +110,8 @@ future<> maintenance_socket_role_manager::remove_attribute(std::string_view role return operation_not_supported_exception("REMOVE ATTRIBUTE"); } +future> maintenance_socket_role_manager::describe_role_grants() { + return operation_not_supported_exception>("DESCRIBE SCHEMA WITH INTERNALS"); } + +} // namespace auth diff --git a/auth/maintenance_socket_role_manager.hh b/auth/maintenance_socket_role_manager.hh index f13d885110..0040eb3cf4 100644 --- a/auth/maintenance_socket_role_manager.hh +++ b/auth/maintenance_socket_role_manager.hh @@ -68,6 +68,8 @@ public: virtual future<> set_attribute(std::string_view role_name, std::string_view attribute_name, std::string_view attribute_value, ::service::group0_batch& mc) override; virtual future<> remove_attribute(std::string_view role_name, std::string_view attribute_name, ::service::group0_batch& mc) override; + + virtual future> describe_role_grants() override; }; } diff --git a/auth/password_authenticator.cc b/auth/password_authenticator.cc index dba3b2aaf1..7612213044 100644 --- a/auth/password_authenticator.cc +++ b/auth/password_authenticator.cc @@ -17,8 +17,10 @@ #include #include #include +#include #include "auth/authenticated_user.hh" +#include "auth/authentication_options.hh" #include "auth/common.hh" #include "auth/passwords.hh" #include "auth/roles-metadata.hh" @@ -199,7 +201,7 @@ bool password_authenticator::require_authentication() const { } authentication_option_set password_authenticator::supported_options() const { - return authentication_option_set{authentication_option::password}; + return authentication_option_set{authentication_option::password, authentication_option::salted_hash}; } authentication_option_set password_authenticator::alterable_options() const { @@ -218,28 +220,8 @@ future password_authenticator::authenticate( const sstring username = credentials.at(USERNAME_KEY); const sstring password = credentials.at(PASSWORD_KEY); - // Here was a thread local, explicit cache of prepared statement. In normal execution this is - // fine, but since we in testing set up and tear down system over and over, we'd start using - // obsolete prepared statements pretty quickly. - // Rely on query processing caching statements instead, and lets assume - // that a map lookup string->statement is not gonna kill us much. - const sstring query = seastar::format("SELECT {} FROM {}.{} WHERE {} = ?", - SALTED_HASH, - get_auth_ks_name(_qp), - meta::roles_table::name, - meta::roles_table::role_col_name); try { - const auto res = co_await _qp.execute_internal( - query, - consistency_for_user(username), - internal_distributed_query_state(), - {username}, - cql3::query_processor::cache_internal::yes); - - auto salted_hash = std::optional(); - if (!res->empty()) { - salted_hash = res->one().get_opt(SALTED_HASH); - } + const std::optional salted_hash = co_await get_password_hash(username); if (!salted_hash || !passwords::check(password, *salted_hash)) { throw exceptions::authentication_exception("Username and/or password are incorrect"); } @@ -258,28 +240,45 @@ future password_authenticator::authenticate( } future<> password_authenticator::create(std::string_view role_name, const authentication_options& options, ::service::group0_batch& mc) { - if (!options.password) { + // When creating a role with the usual `CREATE ROLE` statement, turns the underlying `PASSWORD` + // into the corresponding hash. + // When creating a role with `CREATE ROLE WITH SALTED HASH`, simply extracts the `SALTED HASH`. + auto maybe_hash = options.credentials.transform([&] (const auto& creds) -> sstring { + return std::visit(make_visitor( + [&] (const password_option& opt) { + return passwords::hash(opt.password, rng_for_salt); + }, + [] (const salted_hash_option& opt) { + return opt.salted_hash; + } + ), creds); + }); + + // Neither `PASSWORD`, nor `SALTED HASH` has been specified. + if (!maybe_hash) { co_return; } + const auto query = update_row_query(); if (legacy_mode(_qp)) { co_await _qp.execute_internal( query, consistency_for_user(role_name), internal_distributed_query_state(), - {passwords::hash(*options.password, rng_for_salt), sstring(role_name)}, + {std::move(*maybe_hash), sstring(role_name)}, cql3::query_processor::cache_internal::no).discard_result(); } else { - co_await collect_mutations(_qp, mc, query, - {passwords::hash(*options.password, rng_for_salt), sstring(role_name)}); + co_await collect_mutations(_qp, mc, query, {std::move(*maybe_hash), sstring(role_name)}); } } future<> password_authenticator::alter(std::string_view role_name, const authentication_options& options, ::service::group0_batch& mc) { - if (!options.password) { + if (!options.credentials) { co_return; } + const auto password = std::get(*options.credentials).password; + const sstring query = seastar::format("UPDATE {}.{} SET {} = ? WHERE {} = ?", get_auth_ks_name(_qp), meta::roles_table::name, @@ -290,11 +289,11 @@ future<> password_authenticator::alter(std::string_view role_name, const authent query, consistency_for_user(role_name), internal_distributed_query_state(), - {passwords::hash(*options.password, rng_for_salt), sstring(role_name)}, + {passwords::hash(password, rng_for_salt), sstring(role_name)}, cql3::query_processor::cache_internal::no).discard_result(); } else { co_await collect_mutations(_qp, mc, query, - {passwords::hash(*options.password, rng_for_salt), sstring(role_name)}); + {passwords::hash(password, rng_for_salt), sstring(role_name)}); } } @@ -319,6 +318,36 @@ future password_authenticator::query_custom_options(std::string_ return make_ready_future(); } +bool password_authenticator::uses_password_hashes() const { + return true; +} + +future> password_authenticator::get_password_hash(std::string_view role_name) const { + // Here was a thread local, explicit cache of prepared statement. In normal execution this is + // fine, but since we in testing set up and tear down system over and over, we'd start using + // obsolete prepared statements pretty quickly. + // Rely on query processing caching statements instead, and lets assume + // that a map lookup string->statement is not gonna kill us much. + const sstring query = seastar::format("SELECT {} FROM {}.{} WHERE {} = ?", + SALTED_HASH, + get_auth_ks_name(_qp), + meta::roles_table::name, + meta::roles_table::role_col_name); + + const auto res = co_await _qp.execute_internal( + query, + consistency_for_user(role_name), + internal_distributed_query_state(), + {role_name}, + cql3::query_processor::cache_internal::yes); + + if (res->empty()) { + co_return std::nullopt; + } + + co_return res->one().get_opt(SALTED_HASH); +} + const resource_set& password_authenticator::protected_resources() const { static const resource_set resources({make_data_resource(meta::legacy::AUTH_KS, meta::roles_table::name)}); return resources; diff --git a/auth/password_authenticator.hh b/auth/password_authenticator.hh index 52e7e7ae8a..99238ab992 100644 --- a/auth/password_authenticator.hh +++ b/auth/password_authenticator.hh @@ -72,6 +72,10 @@ public: virtual future query_custom_options(std::string_view role_name) const override; + virtual bool uses_password_hashes() const override; + + virtual future> get_password_hash(std::string_view role_name) const override; + virtual const resource_set& protected_resources() const override; virtual ::shared_ptr new_sasl_challenge() const override; diff --git a/auth/role_manager.hh b/auth/role_manager.hh index a7e03dc4e2..ed09c858f7 100644 --- a/auth/role_manager.hh +++ b/auth/role_manager.hh @@ -18,6 +18,7 @@ #include #include "auth/resource.hh" +#include "cql3/description.hh" #include "seastarx.hh" #include "exceptions/exceptions.hh" #include "service/raft/raft_group0_client.hh" @@ -195,5 +196,8 @@ public: /// \note: This is a no-op if the role does not have the named attribute set. /// virtual future<> remove_attribute(std::string_view role_name, std::string_view attribute_name, ::service::group0_batch& mc) = 0; + + /// Produces descriptions that can be used to restore the role grants. + virtual future> describe_role_grants() = 0; }; } diff --git a/auth/service.cc b/auth/service.cc index 25032a2f33..abca589558 100644 --- a/auth/service.cc +++ b/auth/service.cc @@ -8,6 +8,8 @@ #include #include +#include "auth/authentication_options.hh" +#include "auth/authorizer.hh" #include "auth/resource.hh" #include "auth/service.hh" @@ -25,7 +27,9 @@ #include "auth/role_or_anonymous.hh" #include "cql3/functions/functions.hh" #include "cql3/query_processor.hh" +#include "cql3/description.hh" #include "cql3/untyped_result_set.hh" +#include "cql3/util.hh" #include "db/config.hh" #include "db/consistency_level_type.hh" #include "db/functions/function_name.hh" @@ -33,6 +37,7 @@ #include "schema/schema_fwd.hh" #include #include +#include #include "service/migration_manager.hh" #include "service/raft/raft_group0_client.hh" #include "timestamp.hh" @@ -322,8 +327,11 @@ static void validate_authentication_options_are_supported( } }; - if (options.password) { - check(authentication_option::password); + if (options.credentials) { + std::visit(make_visitor( + [&] (const password_option&) { check(authentication_option::password); }, + [&] (const salted_hash_option&) { check(authentication_option::salted_hash); } + ), *options.credentials); } if (options.options) { @@ -418,6 +426,223 @@ future service::exists(const resource& r) const { return make_ready_future(false); } +future> service::describe_roles(bool with_salted_hashes) { + std::vector result{}; + + const auto roles = co_await _role_manager->query_all(); + result.reserve(roles.size()); + + const bool authenticator_uses_password_hashes = _authenticator->uses_password_hashes(); + + auto produce_create_statement = [with_salted_hashes] (const sstring& formatted_role_name, + const std::optional& maybe_salted_hash, bool can_login, bool is_superuser) { + // Even after applying formatting to a role, `formatted_role_name` can only equal `meta::DEFAULT_SUPER_NAME` + // if the original identifier was equal to it. + const sstring role_part = formatted_role_name == meta::DEFAULT_SUPERUSER_NAME + ? seastar::format("IF NOT EXISTS {}", formatted_role_name) + : formatted_role_name; + + const sstring with_salted_hash_part = with_salted_hashes && maybe_salted_hash + // `K_PASSWORD` in Scylla's CQL grammar requires that passwords be quoted + // with single quotation marks. + ? seastar::format("WITH SALTED HASH = {} AND", cql3::util::single_quote(*maybe_salted_hash)) + : "WITH"; + + return seastar::format("CREATE ROLE {} {} LOGIN = {} AND SUPERUSER = {};", + role_part, with_salted_hash_part, can_login, is_superuser); + }; + + for (const auto& role : roles) { + const sstring formatted_role_name = cql3::util::maybe_quote(role); + + std::optional maybe_salted_hash; + if (authenticator_uses_password_hashes) { + maybe_salted_hash = co_await _authenticator->get_password_hash(role); + } + + const bool can_login = co_await _role_manager->can_login(role); + const bool is_superuser = co_await _role_manager->is_superuser(role); + + result.push_back(cql3::description { + // Roles do not belong to any keyspace. + .keyspace = std::nullopt, + .type = "role", + .name = role, + .create_statement = produce_create_statement(formatted_role_name, maybe_salted_hash, can_login, is_superuser) + }); + } + + std::ranges::sort(result, std::less<>{}, std::mem_fn(&cql3::description::name)); + + co_return result; +} + +// The function doesn't assume anything about `role`. +static sstring describe_data_resource(const permission& perm, const resource& r, std::string_view role) { + const auto permission = permissions::to_string(perm); + const auto formatted_role = cql3::util::maybe_quote(role); + + const auto view = data_resource_view(r); + const auto maybe_ks = view.keyspace(); + const auto maybe_cf = view.table(); + + // The documentation says: + // + // Both keyspace and table names consist of only alphanumeric characters, cannot be empty, + // and are limited in size to 48 characters (that limit exists mostly to avoid filenames, + // which may include the keyspace and table name, to go over the limits of certain file systems). + // By default, keyspace and table names are case insensitive (myTable is equivalent to mytable), + // but case sensitivity can be forced by using double-quotes ("myTable" is different from mytable). + // + // That's why we wrap identifiers with quotation marks below. + + if (!maybe_ks) { + return seastar::format("GRANT {} ON ALL KEYSPACES TO {};", permission, formatted_role); + } + const auto ks = cql3::util::maybe_quote(*maybe_ks); + + if (!maybe_cf) { + return seastar::format("GRANT {} ON KEYSPACE {} TO {};", permission, ks, formatted_role); + } + const auto cf = cql3::util::maybe_quote(*maybe_cf); + + return seastar::format("GRANT {} ON {}.{} TO {};", permission, ks, cf, formatted_role); +} + +// The function doesn't assume anything about `role`. +static sstring describe_role_resource(const permission& perm, const resource& r, std::string_view role) { + const auto permission = permissions::to_string(perm); + const auto formatted_role = cql3::util::maybe_quote(role); + + const auto view = role_resource_view(r); + const auto maybe_target_role = view.role(); + + if (!maybe_target_role) { + return seastar::format("GRANT {} ON ALL ROLES TO {};", permission, formatted_role); + } + return seastar::format("GRANT {} ON ROLE {} TO {};", permission, cql3::util::maybe_quote(*maybe_target_role), formatted_role); +} + +// The function doesn't assume anything about `role`. +static sstring describe_udf_resource(const permission& perm, const resource& r, std::string_view role) { + const auto permission = permissions::to_string(perm); + const auto formatted_role = cql3::util::maybe_quote(role); + + const auto view = functions_resource_view(r); + const auto maybe_ks = view.keyspace(); + const auto maybe_fun_sig = view.function_signature(); + const auto maybe_fun_name = view.function_name(); + const auto maybe_fun_args = view.function_args(); + + // The documentation says: + // + // Both keyspace and table names consist of only alphanumeric characters, cannot be empty, + // and are limited in size to 48 characters (that limit exists mostly to avoid filenames, + // which may include the keyspace and table name, to go over the limits of certain file systems). + // By default, keyspace and table names are case insensitive (myTable is equivalent to mytable), + // but case sensitivity can be forced by using double-quotes ("myTable" is different from mytable). + // + // That's why we wrap identifiers with quotation marks below. + + if (!maybe_ks) { + return seastar::format("GRANT {} ON ALL FUNCTIONS TO {};", permission, formatted_role); + } + const auto ks = cql3::util::maybe_quote(*maybe_ks); + + if (!maybe_fun_sig && !maybe_fun_name) { + return seastar::format("GRANT {} ON ALL FUNCTIONS IN KEYSPACE {} TO {};", permission, ks, formatted_role); + } + + if (maybe_fun_name) { + SCYLLA_ASSERT(maybe_fun_args); + + const auto fun_name = cql3::util::maybe_quote(*maybe_fun_name); + const auto fun_args_range = *maybe_fun_args | std::views::transform([] (const auto& fun_arg) { + return cql3::util::maybe_quote(fun_arg); + }); + + return seastar::format("GRANT {} ON FUNCTION {}.{}({}) TO {};", + permission, ks, fun_name, fmt::join(fun_args_range, ", "), formatted_role); + } + + SCYLLA_ASSERT(maybe_fun_sig); + + auto [fun_name, fun_args] = decode_signature(*maybe_fun_sig); + fun_name = cql3::util::maybe_quote(fun_name); + + // We don't call `cql3::util::maybe_quote` later because `cql3_type_name_without_frozen` already guarantees + // that the type will be wrapped within double quotation marks if it's necessary. + auto parsed_fun_args = fun_args | std::views::transform([] (const data_type& dt) { + return dt->without_reversed().cql3_type_name_without_frozen(); + }); + + return seastar::format("GRANT {} ON FUNCTION {}.{}({}) TO {};", + permission, ks, fun_name, fmt::join(parsed_fun_args, ", "), formatted_role); +} + +// The function doesn't assume anything about `role`. +static sstring describe_resource_kind(const permission& perm, const resource& r, std::string_view role) { + switch (r.kind()) { + case resource_kind::data: + return describe_data_resource(perm, r, role); + case resource_kind::role: + return describe_role_resource(perm, r, role); + case resource_kind::service_level: + on_internal_error(log, "Granting permissions for service levels is not supported"); + case resource_kind::functions: + return describe_udf_resource(perm, r, role); + } +} + +future> service::describe_permissions() const { + std::vector result{}; + + const auto permission_list = co_await std::invoke([&] -> future> { + try { + co_return co_await _authorizer->list_all(); + } catch (const unsupported_authorization_operation&) { + // If Scylla uses AllowAllAuthorizer, permissions do not exist and the corresponding authorizer + // will throw an exception when trying to access them. + co_return std::vector{}; + } + }); + + for (const auto& permissions : permission_list) { + for (const auto& permission : permissions.permissions) { + result.push_back(cql3::description { + // Permission grants do not belong to any keyspace. + .keyspace = std::nullopt, + .type = "grant_permission", + .name = permissions.role_name, + .create_statement = describe_resource_kind(permission, permissions.resource, permissions.role_name) + }); + } + + co_await coroutine::maybe_yield(); + } + + std::ranges::sort(result, std::less<>{}, [] (const cql3::description& desc) noexcept { + return std::make_tuple(std::ref(desc.name), std::ref(*desc.create_statement)); + }); + + co_return result; +} + +future> service::describe_auth(bool with_salted_hashes) { + auto role_descs = co_await describe_roles(with_salted_hashes); + auto role_grant_descs = co_await _role_manager->describe_role_grants(); + auto permission_descs = co_await describe_permissions(); + + auto join_vectors = [] (std::vector& v1, std::vector&& v2) { + v1.insert(v1.end(), std::make_move_iterator(v2.begin()), std::make_move_iterator(v2.end())); + }; + + join_vectors(role_descs, std::move(role_grant_descs)); + join_vectors(role_descs, std::move(permission_descs)); + + co_return role_descs; +} + // // Free functions. // diff --git a/auth/service.hh b/auth/service.hh index b37880a7b2..469b46a447 100644 --- a/auth/service.hh +++ b/auth/service.hh @@ -23,6 +23,7 @@ #include "auth/permissions_cache.hh" #include "auth/role_manager.hh" #include "auth/common.hh" +#include "cql3/description.hh" #include "seastarx.hh" #include "service/raft/raft_group0_client.hh" #include "utils/observable.hh" @@ -174,6 +175,12 @@ public: future exists(const resource&) const; + /// + /// Produces descriptions that can be used to restore the state of auth. That encompasses + /// roles, role grants, and permission grants. + /// + future> describe_auth(bool with_salted_hashes); + authenticator& underlying_authenticator() const { return *_authenticator; } @@ -197,6 +204,9 @@ public: private: future<> create_legacy_keyspace_if_missing(::service::migration_manager& mm) const; future has_superuser(std::string_view role_name, const role_set& roles) const; + + future> describe_roles(bool with_salted_hashes); + future> describe_permissions() const; }; future has_superuser(const service&, const authenticated_user&); diff --git a/auth/standard_role_manager.cc b/auth/standard_role_manager.cc index 0ed7c28f1f..51f2a8f571 100644 --- a/auth/standard_role_manager.cc +++ b/auth/standard_role_manager.cc @@ -24,7 +24,9 @@ #include "auth/role_manager.hh" #include "auth/roles-metadata.hh" #include "cql3/query_processor.hh" +#include "cql3/description.hh" #include "cql3/untyped_result_set.hh" +#include "cql3/util.hh" #include "db/consistency_level_type.hh" #include "exceptions/exceptions.hh" #include "log.hh" @@ -701,4 +703,33 @@ future<> standard_role_manager::remove_attribute(std::string_view role_name, std {sstring(role_name), sstring(attribute_name)}); } } + +future> standard_role_manager::describe_role_grants() { + std::vector result{}; + + const auto grants = co_await query_all_directly_granted(); + result.reserve(grants.size()); + + for (const auto& [grantee_role, granted_role] : grants) { + const auto formatted_grantee = cql3::util::maybe_quote(grantee_role); + const auto formatted_granted = cql3::util::maybe_quote(granted_role); + + result.push_back(cql3::description { + // Role grants do not belong to any keyspace. + .keyspace = std::nullopt, + .type = "grant_role", + .name = granted_role, + .create_statement = seastar::format("GRANT {} TO {};", formatted_granted, formatted_grantee) + }); + + co_await coroutine::maybe_yield(); + } + + std::ranges::sort(result, std::less<>{}, [] (const cql3::description& desc) noexcept { + return std::make_tuple(std::ref(desc.name), std::ref(*desc.create_statement)); + }); + + co_return result; } + +} // namespace auth diff --git a/auth/standard_role_manager.hh b/auth/standard_role_manager.hh index 6a5002cc33..831b6c650c 100644 --- a/auth/standard_role_manager.hh +++ b/auth/standard_role_manager.hh @@ -17,6 +17,7 @@ #include #include +#include "cql3/description.hh" #include "seastarx.hh" #include "service/raft/raft_group0_client.hh" @@ -79,6 +80,8 @@ public: virtual future<> remove_attribute(std::string_view role_name, std::string_view attribute_name, ::service::group0_batch& mc) override; + virtual future> describe_role_grants() override; + private: enum class membership_change { add, remove }; @@ -97,4 +100,4 @@ private: future<> modify_membership(std::string_view role_name, std::string_view grantee_name, membership_change, ::service::group0_batch& mc); }; -} +} // namespace auth diff --git a/auth/transitional.cc b/auth/transitional.cc index aba243dabe..ab3ec7f0e3 100644 --- a/auth/transitional.cc +++ b/auth/transitional.cc @@ -103,6 +103,14 @@ public: return _authenticator->query_custom_options(role_name); } + virtual bool uses_password_hashes() const override { + return _authenticator->uses_password_hashes(); + } + + virtual future> get_password_hash(std::string_view role_name) const override { + return _authenticator->get_password_hash(role_name); + } + virtual const resource_set& protected_resources() const override { return _authenticator->protected_resources(); } diff --git a/bytes.hh b/bytes.hh index b97cccee94..7dcb7f3dbe 100644 --- a/bytes.hh +++ b/bytes.hh @@ -16,13 +16,10 @@ #include #include #include +#include "bytes_fwd.hh" #include "utils/mutable_view.hh" #include "utils/simple_hashers.hh" -using bytes = basic_sstring; -using bytes_view = std::basic_string_view; -using bytes_mutable_view = basic_mutable_view; -using bytes_opt = std::optional; using sstring_view = std::string_view; inline bytes to_bytes(bytes&& b) { diff --git a/bytes_fwd.hh b/bytes_fwd.hh new file mode 100644 index 0000000000..d551779657 --- /dev/null +++ b/bytes_fwd.hh @@ -0,0 +1,20 @@ +/* + * Copyright (C) 2024-present ScyllaDB + */ + +/* + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +#pragma once + +#include + +#include "utils/mutable_view.hh" + +using namespace seastar; + +using bytes = basic_sstring; +using bytes_view = std::basic_string_view; +using bytes_mutable_view = basic_mutable_view; +using bytes_opt = std::optional; diff --git a/configure.py b/configure.py index 7b3e20fe45..d8178ccfc5 100755 --- a/configure.py +++ b/configure.py @@ -890,6 +890,7 @@ scylla_core = (['message/messaging_service.cc', 'cql3/attributes.cc', 'cql3/cf_name.cc', 'cql3/cql3_type.cc', + 'cql3/description.cc', 'cql3/operation.cc', 'cql3/index_name.cc', 'cql3/keyspace_element_name.cc', diff --git a/cql3/CMakeLists.txt b/cql3/CMakeLists.txt index f9c72f5a9f..60faf02167 100644 --- a/cql3/CMakeLists.txt +++ b/cql3/CMakeLists.txt @@ -32,6 +32,7 @@ target_sources(cql3 attributes.cc cf_name.cc cql3_type.cc + description.cc operation.cc index_name.cc keyspace_element_name.cc diff --git a/cql3/Cql.g b/cql3/Cql.g index 981a265980..927fe812a1 100644 --- a/cql3/Cql.g +++ b/cql3/Cql.g @@ -1340,6 +1340,7 @@ roleOptions[cql3::role_options& opts] roleOption[cql3::role_options& opts] : K_PASSWORD '=' v=STRING_LITERAL { opts.password = $v.text; } + | K_SALTED K_HASH '=' v=STRING_LITERAL { opts.salted_hash = $v.text; } | K_OPTIONS '=' m=mapLiteral { opts.options = convert_property_map(m); } | K_SUPERUSER '=' b=BOOLEAN { opts.is_superuser = convert_boolean_literal($b.text); } | K_LOGIN '=' b=BOOLEAN { opts.can_login = convert_boolean_literal($b.text); } @@ -1469,6 +1470,7 @@ describeStatement returns [std::unique_ptr keyspace; sstring generic_name = ""; + bool with_salted_hashes = false; } : ( K_DESCRIBE | K_DESC ) ( (K_CLUSTER) => K_CLUSTER { $stmt = cql3::statements::raw::describe_statement::cluster(); } @@ -1495,7 +1497,8 @@ describeStatement returns [std::unique_ptrwith_internals_details(); } )? + ( K_WITH K_INTERNALS (K_AND K_PASSWORDS { with_salted_hashes = true; })? + { $stmt->with_internals_details(with_salted_hashes); } )? ; /** DEFINITIONS **/ @@ -2071,6 +2074,9 @@ basic_unreserved_keyword returns [sstring str] | K_NOLOGIN | K_OPTIONS | K_PASSWORD + | K_PASSWORDS + | K_SALTED + | K_HASH | K_EXISTS | K_CUSTOM | K_TRIGGER @@ -2231,6 +2237,9 @@ K_ROLES: R O L E S; K_SUPERUSER: S U P E R U S E R; K_NOSUPERUSER: N O S U P E R U S E R; K_PASSWORD: P A S S W O R D; +K_PASSWORDS: P A S S W O R D S; +K_SALTED: S A L T E D; +K_HASH: H A S H; K_LOGIN: L O G I N; K_NOLOGIN: N O L O G I N; K_OPTIONS: O P T I O N S; diff --git a/cql3/cql3_type.cc b/cql3/cql3_type.cc index 9eb1b8ef48..1e660b2c70 100644 --- a/cql3/cql3_type.cc +++ b/cql3/cql3_type.cc @@ -422,7 +422,7 @@ cql3_type::values() { namespace util { -sstring maybe_quote(const sstring& identifier) { +sstring maybe_quote(const std::string_view identifier) { // quote empty string if (identifier.empty()) { return "\"\""; @@ -451,7 +451,7 @@ sstring maybe_quote(const sstring& identifier) { try { // In general it's not a good idea to use the default dialect, but for parsing an identifier, it's okay. cql3::util::do_with_parser(identifier, dialect{}, std::mem_fn(&cql3_parser::CqlParser::cident)); - return identifier; + return sstring{identifier}; } catch(exceptions::syntax_exception&) { // This alphanumeric string is not a valid identifier, so fall // through to have it quoted: @@ -470,7 +470,7 @@ sstring maybe_quote(const sstring& identifier) { } template -static sstring quote_with(const sstring& str) { +static sstring quote_with(const std::string_view str) { static const std::string quote_str{C}; // quote empty string @@ -495,11 +495,11 @@ static sstring quote_with(const sstring& str) { return result; } -sstring quote(const sstring& identifier) { +sstring quote(const std::string_view identifier) { return quote_with<'"'>(identifier); } -sstring single_quote(const sstring& str) { +sstring single_quote(const std::string_view str) { return quote_with<'\''>(str); } diff --git a/cql3/description.cc b/cql3/description.cc new file mode 100644 index 0000000000..1275610bd6 --- /dev/null +++ b/cql3/description.cc @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2024-present ScyllaDB + */ + +/* + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +#include "cql3/description.hh" + +#include + +#include "cql3/util.hh" +#include "log.hh" +#include "types/types.hh" + +static logging::logger dlogger{"description"}; + +namespace cql3 { + +std::vector description::serialize(bool serialize_create_statement) const { + std::vector result{}; + result.reserve(serialize_create_statement ? 4 : 3); + + if (keyspace) { + result.push_back(to_bytes(cql3::util::maybe_quote(*keyspace))); + } else { + result.push_back(data_value::make_null(utf8_type).serialize()); + } + + result.push_back(to_bytes(type)); + result.push_back(to_bytes(cql3::util::maybe_quote(name))); + + if (serialize_create_statement && create_statement) { + result.push_back(to_bytes(*create_statement)); + } else if (serialize_create_statement) { + on_internal_error(dlogger, "create_statement field is empty"); + } + + return result; +} + +} // namespace cql3 diff --git a/cql3/description.hh b/cql3/description.hh new file mode 100644 index 0000000000..5bed626680 --- /dev/null +++ b/cql3/description.hh @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2024-present ScyllaDB + */ + +/* + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +#pragma once + +#include +#include + +#include "bytes_fwd.hh" + +#include +#include + +using namespace seastar; + +namespace cql3 { + +/// Tag indicating whether a describe function should return a `cql3::description` +/// with a `create_statement` or not. +using with_create_statement = bool_class; + +/// An option type that functions generating instances of `cql3::description` +/// can be parameterized with. +/// +/// In some cases, the user doesn't need a create statement, so producing it +/// is a waste of time. An example of that could be `DESCRIBE TYPES`. +/// +/// In some other cases, we may want to produce either a less, or a more detailed +/// create statement depending on the context. An example of that is +/// `DESCRIBE SCHEMA [WITH INTERNALS]` that may print additional details of tables +/// when the `WITH INTERNALS` option is used. +/// +/// Some entities can generate `cql3::description`s parameterized by those two +/// characteristics and this type embodies the choice of the user. +enum class describe_option { + NO_STMTS, /// Describe an entity, but don't generate a create statement. + STMTS, /// Describe an entity and generate a create statement. + STMTS_AND_INTERNALS /// Describe an entity and generate a create statement, + /// including internal details. +}; + +/// Type representing an entity that can be restored by performing +/// a SINGLE CQL query. It can correspond to a tangible object such as +/// a keyspace, a table, or a role, as well as to a more abstract concept +/// like a role grant. +/// +/// Instances of this type correspond to the output of `DESCRIBE` statements. +/// +/// ! Important note: ! +/// ------------------- +/// `description` will wrap `keyspace` and `name` in double quotation marks +/// before serializing them if it's necessary. However, it does NOT apply +/// to `type`. That field is assumed not to need it. If the user uses the interface +/// in a context where that may be needed, they should format `name` on their own. +/// ------------------- +/// +/// See scylladb/scylladb#11106 and scylladb/scylladb#18750 for more context. +/// See also `docs/dev/describe_schema.md`. +struct description { + /// The name of the keyspace the entity belongs to. + /// Empty optional if and only if the entity does not belong to any keyspace. + std::optional keyspace; + /// The name of the type of an entity, e.g. a role may be of type: role + sstring type; + /// The name of the entity itself, e.g. a keyspace of name `ks` will be of name: ks + sstring name; + /// CQL statement that can be used to restore the entity. + std::optional create_statement; + + /// Serialize the description to represent multiple UTF-8 columns. + /// The number of columns will be equal to 4 unless `serialize_create_statement` + /// is `false`, in which case that number will be equal to 3. + /// + /// If `keyspace.has_value() == false`, a `null` will be printed in its place. + /// + /// Precondition: if `serialize_create_statement` is true, then `create_statement.has_value()` + /// is also true. + std::vector serialize(bool serialize_create_statement = true) const; +}; + +} // namespace cql3 diff --git a/cql3/functions/aggregate_fcts.cc b/cql3/functions/aggregate_fcts.cc index a482a94be2..7460260ed4 100644 --- a/cql3/functions/aggregate_fcts.cc +++ b/cql3/functions/aggregate_fcts.cc @@ -9,6 +9,7 @@ */ #include "bytes.hh" +#include "cql3/description.hh" #include "types/types.hh" #include "types/tuple.hh" #include "cql3/functions/scalar_function.hh" @@ -355,34 +356,49 @@ user_aggregate::user_aggregate(function_name fname, bytes_opt initcond, ::shared bool user_aggregate::has_finalfunc() const { return _agg.state_to_result_function != nullptr; } -std::ostream& user_aggregate::describe(std::ostream& os) const { - auto ks = cql3::util::maybe_quote(name().keyspace); - auto na = cql3::util::maybe_quote(name().name); - - os << "CREATE AGGREGATE " << ks << "." << na << "("; - auto a = arg_types(); - for (size_t i = 0; i < a.size(); i++) { - if (i > 0) { - os << ", "; +description user_aggregate::describe(with_create_statement with_stmt) const { + auto maybe_create_statement = std::invoke([&] -> std::optional { + if (!with_stmt) { + return std::nullopt; } - os << a[i]->cql3_type_name(); - } - os << ")\n"; - os << "SFUNC " << cql3::util::maybe_quote(_agg.aggregation_function->name().name) << "\n" - << "STYPE " << _agg.aggregation_function->return_type()->cql3_type_name(); - if (is_reducible()) { - os << "\n" << "REDUCEFUNC " << cql3::util::maybe_quote(_agg.state_reduction_function->name().name); - } - if (has_finalfunc()) { - os << "\n" << "FINALFUNC " << cql3::util::maybe_quote(_agg.state_to_result_function->name().name); - } - if (_agg.initial_state) { - os << "\n" << "INITCOND " << _agg.aggregation_function->return_type()->deserialize(bytes_view(*_agg.initial_state)).to_parsable_string(); - } - os << ";"; + auto ks = cql3::util::maybe_quote(name().keyspace); + auto na = cql3::util::maybe_quote(name().name); - return os; + std::ostringstream os; + + os << "CREATE AGGREGATE " << ks << "." << na << "("; + auto a = arg_types(); + for (size_t i = 0; i < a.size(); i++) { + if (i > 0) { + os << ", "; + } + os << a[i]->cql3_type_name(); + } + os << ")\n"; + + os << "SFUNC " << cql3::util::maybe_quote(_agg.aggregation_function->name().name) << "\n" + << "STYPE " << _agg.aggregation_function->return_type()->cql3_type_name(); + if (is_reducible()) { + os << "\n" << "REDUCEFUNC " << cql3::util::maybe_quote(_agg.state_reduction_function->name().name); + } + if (has_finalfunc()) { + os << "\n" << "FINALFUNC " << cql3::util::maybe_quote(_agg.state_to_result_function->name().name); + } + if (_agg.initial_state) { + os << "\n" << "INITCOND " << _agg.aggregation_function->return_type()->deserialize(bytes_view(*_agg.initial_state)).to_parsable_string(); + } + os << ";"; + + return std::move(os).str(); + }); + + return description { + .keyspace = name().keyspace, + .type = "aggregate", + .name = name().name, + .create_statement = std::move(maybe_create_statement) + }; } shared_ptr diff --git a/cql3/functions/user_aggregate.hh b/cql3/functions/user_aggregate.hh index 5200216c1e..ba930ab7bc 100644 --- a/cql3/functions/user_aggregate.hh +++ b/cql3/functions/user_aggregate.hh @@ -9,22 +9,19 @@ #pragma once #include "scalar_function.hh" -#include "data_dictionary/keyspace_element.hh" +#include "cql3/description.hh" #include "cql3/functions/function_name.hh" #include "db/functions/aggregate_function.hh" namespace cql3 { namespace functions { -class user_aggregate : public db::functions::aggregate_function, public data_dictionary::keyspace_element { +class user_aggregate : public db::functions::aggregate_function { public: user_aggregate(function_name fname, bytes_opt initcond, ::shared_ptr sfunc, ::shared_ptr reducefunc, ::shared_ptr finalfunc); bool has_finalfunc() const; - virtual sstring keypace_name() const override { return name().keyspace; } - virtual sstring element_name() const override { return name().name; } - virtual sstring element_type() const override { return "aggregate"; } - virtual std::ostream& describe(std::ostream& os) const override; + description describe(with_create_statement) const; seastar::shared_ptr sfunc() const { return _agg.aggregation_function; diff --git a/cql3/functions/user_function.cc b/cql3/functions/user_function.cc index 2bae075ff6..51dc87b93f 100644 --- a/cql3/functions/user_function.cc +++ b/cql3/functions/user_function.cc @@ -7,6 +7,7 @@ */ #include "user_function.hh" +#include "cql3/description.hh" #include "cql3/util.hh" #include "log.hh" #include "lang/wasm.hh" @@ -66,32 +67,45 @@ bytes_opt user_function::execute(std::span parameters) { }); } -std::ostream& user_function::describe(std::ostream& os) const { - auto ks = cql3::util::maybe_quote(name().keyspace); - auto na = cql3::util::maybe_quote(name().name); - - os << "CREATE FUNCTION " << ks << "." << na << "("; - for (size_t i = 0; i < _arg_names.size(); i++) { - if (i > 0) { - os << ", "; +description user_function::describe(with_create_statement with_stmt) const { + auto maybe_create_statement = std::invoke([&] -> std::optional { + if (!with_stmt) { + return std::nullopt; } - os << _arg_names[i] << " " << _arg_types[i]->cql3_type_name(); - } - os << ")\n"; - if (_called_on_null_input) { - os << "CALLED"; - } else { - os << "RETURNS NULL"; - } - os << " ON NULL INPUT\n" - << "RETURNS " << _return_type->cql3_type_name() << "\n" - << "LANGUAGE " << _language << "\n" - << "AS $$\n" - << _body << "\n" - << "$$;"; + auto ks = cql3::util::maybe_quote(name().keyspace); + auto na = cql3::util::maybe_quote(name().name); - return os; + std::ostringstream os; + + os << "CREATE FUNCTION " << ks << "." << na << "("; + for (size_t i = 0; i < _arg_names.size(); i++) { + if (i > 0) { + os << ", "; + } + os << _arg_names[i] << " " << _arg_types[i]->cql3_type_name(); + } + os << ")\n"; + + if (_called_on_null_input) { + os << "CALLED"; + } else { + os << "RETURNS NULL"; + } + os << " ON NULL INPUT\n" + << "RETURNS " << _return_type->cql3_type_name() << "\n" + << "LANGUAGE " << _language << "\n" + << "AS $$" << _body << "$$;"; + + return std::move(os).str(); + }); + + return description { + .keyspace = name().keyspace, + .type = "function", + .name = name().name, + .create_statement = std::move(maybe_create_statement) + }; } } diff --git a/cql3/functions/user_function.hh b/cql3/functions/user_function.hh index 0c4a1d8ba8..1ce36848a3 100644 --- a/cql3/functions/user_function.hh +++ b/cql3/functions/user_function.hh @@ -11,16 +11,16 @@ #pragma once #include "abstract_function.hh" +#include "cql3/description.hh" #include "scalar_function.hh" #include "lang/lua.hh" #include "lang/wasm.hh" -#include "data_dictionary/keyspace_element.hh" namespace cql3 { namespace functions { -class user_function final : public abstract_function, public scalar_function, public data_dictionary::keyspace_element { +class user_function final : public abstract_function, public scalar_function { public: struct lua_context { sstring bitcode; @@ -61,10 +61,7 @@ public: virtual bool requires_thread() const override; virtual bytes_opt execute(std::span parameters) override; - virtual sstring keypace_name() const override { return name().keyspace; } - virtual sstring element_name() const override { return name().name; } - virtual sstring element_type() const override { return "function"; } - virtual std::ostream& describe(std::ostream& os) const override; + description describe(with_create_statement) const; }; } diff --git a/cql3/role_options.hh b/cql3/role_options.hh index 29038fd852..f81e46421f 100644 --- a/cql3/role_options.hh +++ b/cql3/role_options.hh @@ -13,6 +13,7 @@ struct role_options final { std::optional is_superuser{}; std::optional can_login{}; std::optional password{}; + std::optional salted_hash{}; // The parser makes a `std::map`, not a `std::unordered_map`. std::optional> options{}; diff --git a/cql3/statements/describe_statement.cc b/cql3/statements/describe_statement.cc index b7923b4fe3..8c9cfa936f 100644 --- a/cql3/statements/describe_statement.cc +++ b/cql3/statements/describe_statement.cc @@ -5,6 +5,7 @@ /* * SPDX-License-Identifier: AGPL-3.0-or-later */ +#include #include #include #include @@ -19,6 +20,7 @@ #include "exceptions/exceptions.hh" #include #include +#include "seastar/coroutine/exception.hh" #include "service/client_state.hh" #include "types/types.hh" #include "cql3/query_processor.hh" @@ -26,6 +28,7 @@ #include "cql3/statements/raw/describe_statement.hh" #include "cql3/statements/describe_statement.hh" #include +#include #include "transport/messages/result_message.hh" #include "transport/messages/result_message_base.hh" #include "service/query_state.hh" @@ -33,20 +36,20 @@ #include "data_dictionary/data_dictionary.hh" #include "service/storage_proxy.hh" #include "cql3/ut_name.hh" -#include "cql3/util.hh" #include "types/map.hh" #include "types/list.hh" #include "cql3/functions/functions.hh" +#include "types/user.hh" #include "view_info.hh" #include "index/secondary_index_manager.hh" #include "cql3/functions/user_function.hh" #include "cql3/functions/user_aggregate.hh" #include "utils/overloaded_functor.hh" -#include "data_dictionary/keyspace_element.hh" #include "db/system_keyspace.hh" #include "db/extensions.hh" #include "utils/sorting.hh" #include "replica/database.hh" +#include "cql3/description.hh" static logging::logger dlogger("describe"); @@ -60,79 +63,24 @@ namespace cql3 { namespace statements { -using keyspace_element = data_dictionary::keyspace_element; - namespace { -// Represents description of keyspace_element -struct description { - sstring _keyspace; - sstring _type; - sstring _name; - std::optional _create_statement; - - // Description without create_statement - description(replica::database& db, const keyspace_element& element) - : _keyspace(util::maybe_quote(element.keypace_name())) - , _type(element.element_type(db)) - , _name(util::maybe_quote(element.element_name())) - , _create_statement(std::nullopt) {} - - // Description with create_statement - description(replica::database& db, const keyspace_element& element, bool with_internals) - : _keyspace(util::maybe_quote(element.keypace_name())) - , _type(element.element_type(db)) - , _name(util::maybe_quote(element.element_name())) - { - std::ostringstream os; - element.describe(db, os, with_internals); - _create_statement = os.str(); - } - - description(replica::database& db, const keyspace_element& element, sstring create_statement) - : _keyspace(util::maybe_quote(element.keypace_name())) - , _type(element.element_type(db)) - , _name(util::maybe_quote(element.element_name())) - , _create_statement(std::move(create_statement)) {} - - std::vector serialize() const { - auto desc = std::vector{ - {to_bytes(_keyspace)}, - {to_bytes(_type)}, - {to_bytes(_name)} - }; - - if (_create_statement) { - desc.push_back({to_bytes(*_create_statement)}); - } - - return desc; - } -}; - -future> generate_descriptions( - replica::database& db, - std::vector> elements, - std::optional with_internals = std::nullopt, - bool sort_by_name = true) -{ - std::vector descs; - descs.reserve(elements.size()); - if (sort_by_name) { - boost::sort(elements, [] (const auto& a, const auto& b) { - return a->element_name() < b->element_name(); - }); - } - - for (auto& e: elements) { - auto desc = (with_internals.has_value()) - ? description(db, *e, *with_internals) - : description(db, *e); - descs.push_back(desc); +template + requires std::is_invocable_r_v> +future> generate_descriptions(const Range& range, const Describer& describer, bool sort_by_name = true) { + std::vector result{}; + result.reserve(std::ranges::size(range)); + for (const auto& element : range) { + result.push_back(describer(element)); co_await coroutine::maybe_yield(); } - co_return descs; + + if (sort_by_name) { + std::ranges::sort(result, std::less<>{}, std::mem_fn(&description::name)); + } + + co_return result; } bool is_index(const data_dictionary::database& db, const schema_ptr& schema) { @@ -155,12 +103,6 @@ bool is_index(const data_dictionary::database& db, const schema_ptr& schema) { * since keyspace and name don't identify function uniquely */ - - -description keyspace(replica::database& db, const lw_shared_ptr& ks, bool with_stmt = false) { - return (with_stmt) ? description(db, *ks, true) : description(db, *ks); -} - description type(replica::database& db, const lw_shared_ptr& ks, const sstring& name) { auto udt_meta = ks->user_types(); if (!udt_meta.has_type(to_bytes(name))) { @@ -168,11 +110,11 @@ description type(replica::database& db, const lw_shared_ptr& } auto udt = udt_meta.get_type(to_bytes(name)); - return description(db, *udt, true); + return udt->describe(with_create_statement::yes); } // Because UDTs can depend on each other, we need to sort them topologically -future>> get_sorted_types(const lw_shared_ptr& ks) { +future> get_sorted_types(const lw_shared_ptr& ks) { struct udts_comparator { inline bool operator()(const user_type& a, const user_type& b) const { return a->get_name_as_string() < b->get_name_as_string(); @@ -189,13 +131,15 @@ future>> get_sorted_types(const l } } - auto sorted = co_await utils::topological_sort(all_udts, adjacency); - co_return boost::copy_range>>(sorted); + co_return co_await utils::topological_sort(all_udts, adjacency); } -future> types(replica::database& db, const lw_shared_ptr& ks, bool with_stmt = false) { +future> types(replica::database& db, const lw_shared_ptr& ks, with_create_statement with_stmt) { auto udts = co_await get_sorted_types(ks); - co_return co_await generate_descriptions(db, udts, (with_stmt) ? std::optional(true) : std::nullopt, false); + auto describer = [with_stmt] (const user_type udt) -> cql3::description { + return udt->describe(with_stmt); + }; + co_return co_await generate_descriptions(udts, describer, false); } future> function(replica::database& db, const sstring& ks, const sstring& name) { @@ -204,23 +148,28 @@ future> function(replica::database& db, const sstring& throw exceptions::invalid_request_exception(format("Function '{}' not found in keyspace '{}'", name, ks)); } - auto udfs = boost::copy_range>>(fs | boost::adaptors::transformed([] (const auto& f) { - return dynamic_pointer_cast(f.second); - }) | boost::adaptors::filtered([] (const auto& f) { + auto udfs = fs | boost::adaptors::transformed([] (const auto& f) { + const auto& [function_name, function_ptr] = f; + return dynamic_pointer_cast(function_ptr); + }) | boost::adaptors::filtered([] (shared_ptr f) { return f != nullptr; - })); + }); if (udfs.empty()) { throw exceptions::invalid_request_exception(format("Function '{}' not found in keyspace '{}'", name, ks)); } - co_return co_await generate_descriptions(db, udfs, true); + auto describer = [] (shared_ptr udf) { + return udf->describe(cql3::with_create_statement::yes); + }; + co_return co_await generate_descriptions(udfs, describer, true); } -future> functions(replica::database& db,const sstring& ks, bool with_stmt = false) { +future> functions(replica::database& db,const sstring& ks, with_create_statement with_stmt) { auto udfs = cql3::functions::instance().get_user_functions(ks); - auto elements = boost::copy_range>>(udfs); - - co_return co_await generate_descriptions(db, elements, (with_stmt) ? std::optional(true) : std::nullopt); + auto describer = [with_stmt] (shared_ptr udf) { + return udf->describe(with_stmt); + }; + co_return co_await generate_descriptions(udfs, describer, true); } future> aggregate(replica::database& db, const sstring& ks, const sstring& name) { @@ -229,23 +178,27 @@ future> aggregate(replica::database& db, const sstring& throw exceptions::invalid_request_exception(format("Aggregate '{}' not found in keyspace '{}'", name, ks)); } - auto udas = boost::copy_range>>(fs | boost::adaptors::transformed([] (const auto& f) { + auto udas = fs | boost::adaptors::transformed([] (const auto& f) { return dynamic_pointer_cast(f.second); - }) | boost::adaptors::filtered([] (const auto& f) { + }) | boost::adaptors::filtered([] (shared_ptr f) { return f != nullptr; - })); + }); if (udas.empty()) { throw exceptions::invalid_request_exception(format("Aggregate '{}' not found in keyspace '{}'", name, ks)); } - co_return co_await generate_descriptions(db, udas, true); + auto describer = [] (shared_ptr uda) { + return uda->describe(with_create_statement::yes); + }; + co_return co_await generate_descriptions(udas, describer, true); } -future> aggregates(replica::database& db, const sstring& ks, bool with_stmt = false) { +future> aggregates(replica::database& db, const sstring& ks, with_create_statement with_stmt) { auto udas = functions::instance().get_user_aggregates(ks); - auto elements = boost::copy_range>>(udas); - - co_return co_await generate_descriptions(db, elements, (with_stmt) ? std::optional(true) : std::nullopt); + auto describer = [with_stmt] (shared_ptr uda) { + return uda->describe(with_stmt); + }; + co_return co_await generate_descriptions(udas, describer, true); } description view(const data_dictionary::database& db, const sstring& ks, const sstring& name, bool with_internals) { @@ -254,7 +207,7 @@ description view(const data_dictionary::database& db, const sstring& ks, const s throw exceptions::invalid_request_exception(format("Materialized view '{}' not found in keyspace '{}'", name, ks)); } - return description(db.real_database(), *view->schema(), with_internals); + return view->schema()->describe(db.real_database(), with_internals ? describe_option::STMTS_AND_INTERNALS : describe_option::STMTS); } description index(const data_dictionary::database& db, const sstring& ks, const sstring& name, bool with_internals) { @@ -272,7 +225,7 @@ description index(const data_dictionary::database& db, const sstring& ks, const } } - return description(db.real_database(), **idx, with_internals); + return (**idx).describe(db.real_database(), with_internals ? describe_option::STMTS_AND_INTERNALS : describe_option::STMTS); } // `base_name` should be a table with enabled cdc @@ -286,7 +239,10 @@ std::optional describe_cdc_log_table(const data_dictionary::databas std::ostringstream os; auto schema = table->schema(); schema->describe_alter_with_properties(db.real_database(), os); - return description(db.real_database(), *schema, std::move(os.str())); + + auto schema_desc = schema->describe(db.real_database(), describe_option::NO_STMTS); + schema_desc.create_statement = std::move(os).str(); + return schema_desc; } future> table(const data_dictionary::database& db, const sstring& ks, const sstring& name, bool with_internals) { @@ -305,7 +261,7 @@ future> table(const data_dictionary::database& db, cons std::vector result; // table - result.push_back(description(db.real_database(), *schema, with_internals)); + result.push_back(schema->describe(db.real_database(), with_internals ? describe_option::STMTS_AND_INTERNALS : describe_option::STMTS)); // indexes boost::sort(idxs, [] (const auto& a, const auto& b) { @@ -358,7 +314,7 @@ future> tables(const data_dictionary::database& db, con } co_return boost::copy_range>(tables | boost::adaptors::transformed([&replica_db] (auto&& t) { - return description(replica_db, *t); + return t->describe(replica_db, describe_option::NO_STMTS); })); } @@ -381,9 +337,9 @@ lw_shared_ptr get_keyspace_metadata(const data_dictionary::da } /** - * Lists `keyspace_element` for given keyspace + * Lists keyspace elements for given keyspace * - * @return vector of `description` for specified `keyspace_element` in specified keyspace. + * @return vector of `description` for the specified keyspace element type in the specified keyspace. Descriptions don't contain create_statements. * @throw `invalid_request_exception` if there is no such keyspace */ @@ -392,11 +348,11 @@ future> list_elements(const data_dictionary::database& auto& replica_db = db.real_database(); switch (element) { - case element_type::type: co_return co_await types(replica_db, ks_meta); - case element_type::function: co_return co_await functions(replica_db, ks); - case element_type::aggregate: co_return co_await aggregates(replica_db, ks); + case element_type::type: co_return co_await types(replica_db, ks_meta, with_create_statement::no); + case element_type::function: co_return co_await functions(replica_db, ks, with_create_statement::no); + case element_type::aggregate: co_return co_await aggregates(replica_db, ks, with_create_statement::no); case element_type::table: co_return co_await tables(db, ks_meta); - case element_type::keyspace: co_return std::vector{keyspace(replica_db, ks_meta)}; //std::vector{description(ks, "keyspace", ks)}; + case element_type::keyspace: co_return std::vector{ks_meta->describe(replica_db, with_create_statement::no)}; case element_type::view: case element_type::index: on_internal_error(dlogger, "listing of views and indexes is unsupported"); @@ -405,9 +361,9 @@ future> list_elements(const data_dictionary::database& /** - * Describe specified `keyspace_element` in given keyspace + * Describe specified keyspace element type in given keyspace * - * @return `description` of specified `keyspace_element` in specified keyspace. + * @return `description` of the specified keyspace element type in the specified keyspace. Description contains create_statement. * @throw `invalid_request_exception` if there is no such keyspace or there is no element with given name */ @@ -422,7 +378,7 @@ future> describe_element(const data_dictionary::databas case element_type::table: co_return co_await table(db, ks, name, with_internals); case element_type::index: co_return std::vector{index(db, ks, name, with_internals)}; case element_type::view: co_return std::vector{view(db, ks, name, with_internals)}; - case element_type::keyspace: co_return std::vector{keyspace(replica_db, ks_meta, true)}; + case element_type::keyspace: co_return std::vector{ks_meta->describe(replica_db, with_create_statement::yes)}; } } @@ -444,10 +400,10 @@ future> describe_all_keyspace_elements(const data_dicti result.insert(result.end(), elements.begin(), elements.end()); }; - result.push_back(keyspace(replica_db, ks_meta, true)); - inserter(co_await types(replica_db, ks_meta, true)); - inserter(co_await functions(replica_db, ks, true)); - inserter(co_await aggregates(replica_db, ks, true)); + result.push_back(ks_meta->describe(replica_db, with_create_statement::yes)); + inserter(co_await types(replica_db, ks_meta, with_create_statement::yes)); + inserter(co_await functions(replica_db, ks, with_create_statement::yes)); + inserter(co_await aggregates(replica_db, ks, with_create_statement::yes)); inserter(co_await tables(db, ks_meta, with_internals)); co_return result; @@ -471,9 +427,9 @@ std::vector> get_element_column_specificatio return col_specs; } -std::vector> serialize_descriptions(std::vector&& descs) { - return boost::copy_range>>(descs | boost::adaptors::transformed([] (const description& desc) { - return desc.serialize(); +std::vector> serialize_descriptions(std::vector&& descs, bool serialize_create_statement = true) { + return boost::copy_range>>(descs | boost::adaptors::transformed([serialize_create_statement] (const description& desc) { + return desc.serialize(serialize_create_statement); })); } @@ -601,9 +557,9 @@ future>> cluster_describe_statement::describe } // SCHEMA DESCRIBE STATEMENT -schema_describe_statement::schema_describe_statement(bool full_schema, bool with_internals) +schema_describe_statement::schema_describe_statement(bool full_schema, bool with_salted_hashes, bool with_internals) : describe_statement() - , _config(schema_desc{full_schema}) + , _config(schema_desc{.full_schema = full_schema, .with_salted_hashes = with_salted_hashes}) , _with_internals(with_internals) {} schema_describe_statement::schema_describe_statement(std::optional keyspace, bool only, bool with_internals) @@ -620,6 +576,16 @@ future>> schema_describe_statement::describe( auto result = co_await std::visit(overloaded_functor{ [&] (const schema_desc& config) -> future> { + auto& auth_service = *client_state.get_auth_service(); + + if (config.with_salted_hashes) { + const auto maybe_user = client_state.user(); + if (!maybe_user || !co_await auth::has_superuser(auth_service, *maybe_user)) { + co_await coroutine::return_exception(exceptions::unauthorized_exception( + "DESCRIBE SCHEMA WITH INTERNALS AND PASSWORDS can only be issued by a superuser")); + } + } + auto keyspaces = config.full_schema ? db.get_all_keyspaces() : db.get_user_keyspaces(); std::vector schema_result; @@ -631,6 +597,18 @@ future>> schema_describe_statement::describe( schema_result.insert(schema_result.end(), ks_result.begin(), ks_result.end()); } + if (_with_internals) { + // The order is important here. We need to first restore auth + // because we can only attach a service level to an existing role. + auto auth_descs = co_await auth_service.describe_auth(config.with_salted_hashes); + auto service_level_descs = co_await client_state.get_service_level_controller().describe_service_levels(); + + schema_result.insert(schema_result.end(), std::make_move_iterator(auth_descs.begin()), + std::make_move_iterator(auth_descs.end())); + schema_result.insert(schema_result.end(), std::make_move_iterator(service_level_descs.begin()), + std::make_move_iterator(service_level_descs.end())); + } + co_return schema_result; }, [&] (const keyspace_desc& config) -> future> { @@ -641,7 +619,7 @@ future>> schema_describe_statement::describe( if (config.only_keyspace) { auto ks_meta = get_keyspace_metadata(db, ks); - auto ks_desc = keyspace(db.real_database(), ks_meta, true); + auto ks_desc = ks_meta->describe(db.real_database(), with_create_statement::yes); co_return std::vector{ks_desc}; } else { @@ -683,7 +661,7 @@ future>> listing_describe_statement::describe co_await coroutine::maybe_yield(); } - co_return serialize_descriptions(std::move(result)); + co_return serialize_descriptions(std::move(result), false); } // ELEMENT DESCRIBE STATEMENT @@ -764,22 +742,34 @@ future>> generic_describe_statement::describe auto uf = functions::instance().find(functions::function_name(ks_name, _name)); if (!uf.empty()) { - auto udfs = boost::copy_range>>(uf | boost::adaptors::transformed([] (const auto& f) { - return dynamic_pointer_cast(f.second); - }) | boost::adaptors::filtered([] (const auto& f) { + auto udfs = uf | boost::adaptors::transformed([] (const auto& f) { + const auto& [function_name, function_ptr] = f; + return dynamic_pointer_cast(function_ptr); + }) | boost::adaptors::filtered([] (shared_ptr f) { return f != nullptr; - })); + }); + + auto udf_describer = [] (shared_ptr udf) { + return udf->describe(with_create_statement::yes); + }; + if (!udfs.empty()) { - co_return serialize_descriptions(co_await generate_descriptions(replica_db, udfs, true)); + co_return serialize_descriptions(co_await generate_descriptions(udfs, udf_describer, true)); } - auto udas = boost::copy_range>>(uf | boost::adaptors::transformed([] (const auto& f) { - return dynamic_pointer_cast(f.second); - }) | boost::adaptors::filtered([] (const auto& f) { + auto udas = uf | boost::adaptors::transformed([] (const auto& f) { + const auto& [function_name, function_ptr] = f; + return dynamic_pointer_cast(function_ptr); + }) | boost::adaptors::filtered([] (shared_ptr f) { return f != nullptr; - })); + }); + + auto uda_describer = [] (shared_ptr uda) { + return uda->describe(with_create_statement::yes); + }; + if (!udas.empty()) { - co_return serialize_descriptions(co_await generate_descriptions(replica_db, udas, true)); + co_return serialize_descriptions(co_await generate_descriptions(udas, uda_describer, true)); } } @@ -792,8 +782,16 @@ using ds = describe_statement; describe_statement::describe_statement(ds::describe_config config) : _config(std::move(config)), _with_internals(false) {} -void describe_statement::with_internals_details() { +void describe_statement::with_internals_details(bool with_salted_hashes) { _with_internals = internals(true); + + if (with_salted_hashes && !std::holds_alternative(_config)) { + throw exceptions::invalid_request_exception{"Option WITH PASSWORDS is only allowed with DESC SCHEMA"}; + } + + if (std::holds_alternative(_config)) { + std::get(_config).with_salted_hashes = with_salted_hashes; + } } std::unique_ptr describe_statement::prepare(data_dictionary::database db, cql_stats &stats) { @@ -803,7 +801,7 @@ std::unique_ptr describe_statement::prepare(data_dictionary: return ::make_shared(); }, [&] (const describe_schema& cfg) -> ::shared_ptr { - return ::make_shared(cfg.full_schema, internals); + return ::make_shared(cfg.full_schema, cfg.with_salted_hashes, internals); }, [&] (const describe_keyspace& cfg) -> ::shared_ptr { return ::make_shared(std::move(cfg.keyspace), cfg.only_keyspace, internals); diff --git a/cql3/statements/describe_statement.hh b/cql3/statements/describe_statement.hh index 88adc4ba0c..ee7b5045f5 100644 --- a/cql3/statements/describe_statement.hh +++ b/cql3/statements/describe_statement.hh @@ -23,7 +23,6 @@ * - generic_describe_statements - DESC * * Keyspace element means: UDT, UDF, UDA, index, view or table - * (see `data_dictionary/keyspace_element.hh`) */ namespace replica { @@ -76,6 +75,7 @@ public: class schema_describe_statement : public describe_statement { struct schema_desc { bool full_schema; + bool with_salted_hashes; }; struct keyspace_desc { std::optional keyspace; @@ -91,7 +91,7 @@ protected: virtual seastar::future>> describe(cql3::query_processor& qp, const service::client_state& client_state) const override; public: - schema_describe_statement(bool full_schema, bool with_internals); + schema_describe_statement(bool full_schema, bool with_salted_hashes, bool with_internals); schema_describe_statement(std::optional keyspace, bool only, bool with_internals); }; diff --git a/cql3/statements/raw/describe_statement.hh b/cql3/statements/raw/describe_statement.hh index 9902838ab8..d8fceec9a9 100644 --- a/cql3/statements/raw/describe_statement.hh +++ b/cql3/statements/raw/describe_statement.hh @@ -43,7 +43,8 @@ private: struct describe_cluster {}; struct describe_schema { - bool full_schema; + bool full_schema = false; + bool with_salted_hashes = false; }; struct describe_keyspace { std::optional keyspace; @@ -76,7 +77,7 @@ private: public: explicit describe_statement(describe_config config); - void with_internals_details(); + void with_internals_details(bool with_salted_hashes); virtual std::unique_ptr prepare(data_dictionary::database db, cql_stats& stats) override; diff --git a/cql3/statements/role-management-statements.cc b/cql3/statements/role-management-statements.cc index fa0367e445..e21ce02b40 100644 --- a/cql3/statements/role-management-statements.cc +++ b/cql3/statements/role-management-statements.cc @@ -37,13 +37,23 @@ using result_message = cql_transport::messages::result_message; using result_message_ptr = ::shared_ptr; static auth::authentication_options extract_authentication_options(const cql3::role_options& options) { + if (options.password && options.salted_hash) { + throw exceptions::syntax_exception("Only one of the options: PASSWORD, SALTED HASH can be provided"); + } + auth::authentication_options authen_options; - authen_options.password = options.password; if (options.options) { authen_options.options = std::unordered_map(options.options->begin(), options.options->end()); } + if (options.salted_hash) { + authen_options.credentials = auth::salted_hash_option {.salted_hash = *options.salted_hash}; + } + if (options.password) { + authen_options.credentials = auth::password_option {.password = *options.password}; + } + return authen_options; } @@ -75,10 +85,18 @@ future<> create_role_statement::check_access(query_processor& qp, const service: return async([this, &state] { state.ensure_has_permission({auth::permission::CREATE, auth::root_role_resource()}).get(); - if (*_options.is_superuser) { - if (!auth::has_superuser(*state.get_auth_service(), *state.user()).get()) { - throw exceptions::unauthorized_exception("Only superusers can create a role with superuser status."); - } + if (!_options.is_superuser && !_options.salted_hash) { + return; + } + + const bool has_superuser = auth::has_superuser(*state.get_auth_service(), *state.user()).get(); + + if (_options.salted_hash && !has_superuser) { + throw exceptions::unauthorized_exception("Only superusers can create a role with a salted hash."); + } + + if (_options.is_superuser.value_or(false) && !has_superuser) { + throw exceptions::unauthorized_exception("Only superusers can create a role with superuser status."); } }); } diff --git a/cql3/util.hh b/cql3/util.hh index fa37b19a21..ee1c815f25 100644 --- a/cql3/util.hh +++ b/cql3/util.hh @@ -69,7 +69,7 @@ std::unique_ptr build_select_statement( /// parser. To avoid this forward-compatibility issue, use quote() instead /// of maybe_quote() - to unconditionally quote an identifier even if it is /// lowercase and not (yet) a keyword. -sstring maybe_quote(const sstring& s); +sstring maybe_quote(const std::string_view s); /// quote() takes an identifier - the name of a column, table or keyspace - /// and transforms it to a string which can be safely used in CQL commands. @@ -80,14 +80,14 @@ sstring maybe_quote(const sstring& s); /// lowercase if not quoted), or when the identifier is one of many CQL /// keywords. But it's allowed - and easier - to just unconditionally /// quote the identifier name in CQL, so that is what this function does does. -sstring quote(const sstring& s); +sstring quote(const std::string_view s); /// single_quote() takes a string and transforms it to a string /// which can be safely used in CQL commands. /// Single quoting involves wrapping the name in single-quotes ('). A sigle-quote /// character itself is quoted by doubling it. /// Single quoting is necessary for dates, IP addresses or string literals. -sstring single_quote(const sstring& s); +sstring single_quote(const std::string_view s); // Check whether timestamp is not too far in the future as this probably // indicates its incorrectness (for example using other units than microseconds). diff --git a/data_dictionary/data_dictionary.cc b/data_dictionary/data_dictionary.cc index f81629c3fe..f08f50be3b 100644 --- a/data_dictionary/data_dictionary.cc +++ b/data_dictionary/data_dictionary.cc @@ -7,6 +7,7 @@ */ #include "data_dictionary.hh" +#include "cql3/description.hh" #include "impl.hh" #include "user_types_metadata.hh" #include "keyspace_metadata.hh" @@ -351,32 +352,47 @@ no_such_column_family::no_such_column_family(std::string_view ks_name, const tab { } -std::ostream& keyspace_metadata::describe(replica::database& db, std::ostream& os, bool with_internals) const { - os << "CREATE KEYSPACE " << cql3::util::maybe_quote(_name) - << " WITH replication = {'class': " << cql3::util::single_quote(_strategy_name); - for (const auto& opt: _strategy_options) { - os << ", " << cql3::util::single_quote(opt.first) << ": " << cql3::util::single_quote(opt.second); - } - if (!_storage_options->is_local_type()) { - os << "} AND storage = {'type': " << cql3::util::single_quote(sstring(_storage_options->type_string())); - for (const auto& e : _storage_options->to_map()) { - os << ", " << cql3::util::single_quote(e.first) << ": " << cql3::util::single_quote(e.second); +cql3::description keyspace_metadata::describe(const replica::database& db, cql3::with_create_statement with_create_statement) const { + auto maybe_create_statement = std::invoke([&] -> std::optional { + if (!with_create_statement) { + return std::nullopt; } - } - os << "} AND durable_writes = " << std::boolalpha << _durable_writes << std::noboolalpha; - if (db.features().tablets) { - if (!_initial_tablets.has_value()) { - os << " AND tablets = {'enabled': false}"; - } else if (_initial_tablets.value() > 0) { - os << " AND tablets = {'initial': " << _initial_tablets.value() << "}"; - } - } - os << ";"; - return os; + std::ostringstream os; + + os << "CREATE KEYSPACE " << cql3::util::maybe_quote(_name) + << " WITH replication = {'class': " << cql3::util::single_quote(_strategy_name); + for (const auto& opt: _strategy_options) { + os << ", " << cql3::util::single_quote(opt.first) << ": " << cql3::util::single_quote(opt.second); + } + if (!_storage_options->is_local_type()) { + os << "} AND storage = {'type': " << cql3::util::single_quote(sstring(_storage_options->type_string())); + for (const auto& e : _storage_options->to_map()) { + os << ", " << cql3::util::single_quote(e.first) << ": " << cql3::util::single_quote(e.second); + } + } + os << "} AND durable_writes = " << std::boolalpha << _durable_writes << std::noboolalpha; + if (db.features().tablets) { + if (!_initial_tablets.has_value()) { + os << " AND tablets = {'enabled': false}"; + } else if (_initial_tablets.value() > 0) { + os << " AND tablets = {'initial': " << _initial_tablets.value() << "}"; + } + } + os << ";"; + + return std::move(os).str(); + }); + + return cql3::description { + .keyspace = name(), + .type = "keyspace", + .name = name(), + .create_statement = std::move(maybe_create_statement) + }; } -} +} // namespace data_dictionary template <> struct fmt::formatter { diff --git a/data_dictionary/keyspace_element.hh b/data_dictionary/keyspace_element.hh deleted file mode 100644 index e7e70bfbcd..0000000000 --- a/data_dictionary/keyspace_element.hh +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (C) 2022-present ScyllaDB - */ - -/* - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -#pragma once - -#include -#include - -namespace replica { -class database; -} - -namespace data_dictionary { - -/** - * `keyspace_element` is a common interface used to describe elements of keyspace. - * It is used in `describe_statement`. - * - * Currently the elements of keyspace are: - * - keyspace - * - user-defined types - * - user-defined functions/aggregates - * - tables, views and indexes -*/ -class keyspace_element { -public: - virtual seastar::sstring keypace_name() const = 0; - virtual seastar::sstring element_name() const = 0; - - // Override one of these element_type() overloads. - virtual seastar::sstring element_type() const { return ""; } - virtual seastar::sstring element_type(replica::database& db) const { return element_type(); } - - // Override one of these describe() overloads. - virtual std::ostream& describe(std::ostream& os) const { return os; } - virtual std::ostream& describe(replica::database& db, std::ostream& os, bool with_internals) const { return describe(os); } -}; - -} diff --git a/data_dictionary/keyspace_metadata.hh b/data_dictionary/keyspace_metadata.hh index 48e4a6d17e..1eed754bcd 100644 --- a/data_dictionary/keyspace_metadata.hh +++ b/data_dictionary/keyspace_metadata.hh @@ -10,14 +10,13 @@ #include #include -#include #include +#include "cql3/description.hh" #include "schema/schema.hh" #include "locator/abstract_replication_strategy.hh" #include "data_dictionary/user_types_metadata.hh" #include "data_dictionary/storage_options.hh" -#include "data_dictionary/keyspace_element.hh" namespace gms { class feature_service; @@ -25,7 +24,7 @@ class feature_service; namespace data_dictionary { -class keyspace_metadata final : public keyspace_element { +class keyspace_metadata final { sstring _name; sstring _strategy_name; locator::replication_strategy_config_options _strategy_options; @@ -95,10 +94,7 @@ public: std::vector tables() const; std::vector views() const; - virtual sstring keypace_name() const override { return name(); } - virtual sstring element_name() const override { return name(); } - virtual sstring element_type() const override { return "keyspace"; } - virtual std::ostream& describe(replica::database& db, std::ostream& os, bool with_internals) const override; + cql3::description describe(const replica::database& db, cql3::with_create_statement) const; }; } diff --git a/docs/cql/cql-extensions.md b/docs/cql/cql-extensions.md index 426cead8fe..8d0a6dd8db 100644 --- a/docs/cql/cql-extensions.md +++ b/docs/cql/cql-extensions.md @@ -454,3 +454,11 @@ To facilitate insight into which values come from which service level, there is ``` For more details, check [Service Levels docs](https://github.com/scylladb/scylla/blob/master/docs/cql/service-levels.rst) + +## DESCRIBE SCHEMA WITH INTERNALS [AND PASSWORDS] + +We extended the semantics of `DESCRIBE SCHEMA WITH INTERNALS`: aside from describing the elements of the schema, +it also describes authentication/authorization and service levels. Additionally, we introduced a new tier of the +statement: `DESCRIBE SCHEMA WITH INTERNALS AND PASSWORDS`, which also includes the information about salted hashes of the roles. + +For more details, see [the article on DESCRIBE SCHEMA](./describe-schema.rst). diff --git a/docs/cql/cqlsh.rst b/docs/cql/cqlsh.rst index da0c698c5a..05ece6a6b9 100644 --- a/docs/cql/cqlsh.rst +++ b/docs/cql/cqlsh.rst @@ -399,8 +399,11 @@ The ``DESCRIBE CLUSTER`` command prints the cluster name and partitioner:: Cluster: Test Cluster Partitioner: Murmur3Partitioner -The ``DESCRIBE SCHEMA`` command prints the DDL statements needed to recreate the entire schema. This is especially -useful for dumping the schema in order to clone a cluster or restore from a backup. +The ``DESCRIBE SCHEMA`` command prints the DDL statements needed to recreate the entire schema. The higher tiers of the statement +can also include information that can be used to restore authentication/authorization and service levels. This is especially +useful for cloning a cluster or restoring its state from a backup. + +See :doc:`a dedicated article ` to learn more. .. _cqlsh-copy-to: diff --git a/docs/cql/describe-schema.rst b/docs/cql/describe-schema.rst new file mode 100644 index 0000000000..e5ffbf04a4 --- /dev/null +++ b/docs/cql/describe-schema.rst @@ -0,0 +1,111 @@ +=============== +DESCRIBE SCHEMA +=============== + +When using ScyllaDB, you may want to back up your data. In case something unexpected happens, you will +be able to restore it. However, you cannot insert data into the void; because of that, you need to +first restore the schema. + +That is the main motivation for an introduced statement in ScyllaDB: ``DESCRIBE SCHEMA``, which is +responsible for producing a sequence of CQL commands that can be executed in order to recreate the schema. + +Aside from elements of schema, like keyspaces and tables, it also allows for restoring entities such as +roles and service levels. + +For more context, see the articles covering :doc:`backing up ` +and :doc:`restoring ` the schema. + +Syntax and semantics +-------------------- + +``DESCRIBE SCHEMA`` comes in three forms that differ in semantics. We will refer to those forms as "tiers". + +* ``DESCRIBE [FULL] SCHEMA``: describe elements of the non-system schema: keyspaces, tables, views, UDTs, etc. + When `FULL` is used, it also includes the elements of the system schema, e.g. system tables. + +* ``DESCRIBE [FULL] SCHEMA WITH INTERNALS``: in addition to the output of the previous tier, the statement + also describes authentication and authorization, as well as service levels. The statements corresponding to + restoring roles do *not* contain any information about their passwords. Aside from that, additional information + about tables, materialized views, and secondary indices may be provided, e.g. the ID of a table. + +* ``DESCRIBE [FULL] SCHEMA WITH INTERNALS AND PASSWORDS``: aside from the information retrieved as part of the + previous tier, the statements corresponding to restoring roles *do* contain information about their + passwords; namely—their salted hashes. For more information regarding salted hashes, see the relevant section + below. + +Instead of ``DESCRIBE``, you can use its shortened form: ``DESC``. + +Output +------ + +As a result of the query, you will obtain a set of rows, each of which consists of four values: + +* ``keyspace_name``: the name of the keyspace the entity is part of, +* ``type``: the type of the entity, +* ``name``: the name of the entity, +* ``create_statement``: the statement that can be used to restore the entity. + +All of those values are always present and represent strings using UTF-8 encoding. +The value ``keyspace_name`` can be equal to ``null`` if the corresponding entity is not part of any keyspace, +e.g. in the case of a role or a service level. + +Required permissions +-------------------- + +No permissions are necessary to execute the first two tiers. However, executing the last tier requires that the user +performing the query be a superuser. + +Restoring the schema by executing the consecutive statements returned by ``DESCRIBE SCHEMA`` should be conducted by +a superuser—some of the statements may require that status. It's advised to use the default superuser and not modify +the schema, roles, permissions, or service levels beforehand. + +Relation to authentication and authorization +-------------------------------------------- + +It's important to note that the information returned by ``DESCRIBE [FULL] SCHEMA WITH INTERNALS [AND PASSWORDS]`` +depends on the currently used authenticator and authorizer. If the used authenticator doesn't use passwords to +authenticate a role, the salted hashes won't be returned even if they're present in the database. That scenario +may happen if, for example, you start using `AllowAllAuthenticator`. + +Similarly, permission grants may not be returned if they're not used, e.g. when ScyllaDB is configured to use +`AllowAllAuthorizer`. + +Since ScyllaDB may also use third party software service for authentication and authorization, the result of the query +will depend on what information that service will provide the database with. + +That's why it's *crucial* for you to take into consideration the current configuration of your cluster. + +Restoring process and its side effects +-------------------------------------- + +When a resource is created, the current role is always granted full permissions to that resource. As a consequence, the role +used to restore the backup will gain permissions to all of the resources that will be recreated. That's one of the reasons +why using a superuser to restore the schema is advised—a superuser has full access to all resources anyway. + +cqlsh support +------------- + + +---------------------------------------------------------+---------------------------+ + | Statement | Required version of cqlsh | + +=========================================================+===========================+ + | ``DESCRIBE [FULL] SCHEMA`` | 6.0.19 | + +---------------------------------------------------------+---------------------------+ + | ``DESCRIBE [FULL] SCHEMA WITH INTERNALS`` | 6.0.19 | + +---------------------------------------------------------+---------------------------+ + | ``DESCRIBE [FULL] SCHEMA WITH INTERNALS AND PASSWORDS`` | 6.0.23 | + +---------------------------------------------------------+---------------------------+ + +Appendix I, salted hashes +------------------------- + +A salted hash is an encrypted form of a password stored by ScyllaDB to authenticate roles. The statements returned +by ``DESCRIBE [FULL] SCHEMA WITH INTERNALS AND PASSWORDS`` corresponding to recreating the roles will be of the +following form: + +.. code-block:: cql + + CREATE ROLE [IF NOT EXISTS] WITH SALTED HASH = '' AND LOGIN = AND SUPERUSER = + +The semantics of this statement is analogous to the regular ``CREATE ROLE`` statement except that it circumvents +the encryption phase of the execution and inserts the salted hash directly into ``system.roles``. You should not use +this statement unless it was returned by ``DESCRIBE SCHEMA``. diff --git a/docs/cql/index.rst b/docs/cql/index.rst index fd07759e7e..b414769b49 100644 --- a/docs/cql/index.rst +++ b/docs/cql/index.rst @@ -22,6 +22,7 @@ CQL Reference mv non-reserved-keywords reserved-keywords + describe-schema service-levels cql-extensions.md diff --git a/docs/cql/non-reserved-keywords.rst b/docs/cql/non-reserved-keywords.rst index 3d28bf8271..b3b974d9e7 100644 --- a/docs/cql/non-reserved-keywords.rst +++ b/docs/cql/non-reserved-keywords.rst @@ -30,6 +30,7 @@ Non-reserved keywords only have meaning in their particular area of context and * FROZEN * FUNCTION * FUNCTIONS +* HASH * INET * INITCOND * INPUT @@ -46,11 +47,13 @@ Non-reserved keywords only have meaning in their particular area of context and * NOSUPERUSER * OPTIONS * PASSWORD +* PASSWORDS * PERMISSION * PERMISSIONS * RETURNS * ROLE * ROLES +* SALTED * SFUNC * SMALLINT * STATIC diff --git a/docs/dev/describe_schema.md b/docs/dev/describe_schema.md new file mode 100644 index 0000000000..8c9db22c36 --- /dev/null +++ b/docs/dev/describe_schema.md @@ -0,0 +1,121 @@ +# Motivation +Scylla clusters keep various kinds of metadata that needs to be global. The list includes but is not limited to: + +* schema, +* authorization/authentication, +* service levels. + +In the past, they were replicated using mechanisms like custom synchronization RPC, while auth and service levels—distributed tables. All of them suffered from consistency issues, e.g. concurrent modification could make them break, some were prone to data loss if a node was lost. To solve those issues, we replaced the mechanisms with the Raft algorithm and now they're managed by group0. + +Since we want to support cluster-wide backup and restore[^1], we need to be able to backup and restore cluster metadata. The currently recommended way of backing-up tables with user data is to take a snapshot, upload the SSTables elswehere and, during restore, download them back to appropriate nodes in the fresh cluster. + +Unfortunately, that doesn't work well with Raft-replicated tables for the following reasons: + +* group0-managed tables are just local system tables and any modifications to them are propagated to all nodes using Raft. That assumes that if there are no unapplied Raft commands, all nodes will have the same state of the table. + + Theoretically, we could make it work if we restored the same SSTables on all nodes, but although it sounds simple, it's not always possible: Scylla Manager cannot transfer SSTables backed-up in one data center to another one. Taking a snapshot on more than one node is not atomic and does not guarantee that the contents of their snapshots will be consistent. In case of any differences between the nodes, there is no repair algorithm that could reconcile their states. The fact that application of group0 commands to tables is not synchronized with the flush on snapshot doesn't help either. + +* Restoring data by putting SSTables into the data directory is potentially unsafe. Nothing prevents the administrator from restoring only some of the backed-up SSTables. Also, group0 generally assumes that all its data was created by relevant code in Scylla and has not been tampered with by the user. It assumes that for all data stored in SSTables, the commitlog, etc. + +To summarize, it's very easy to make a mistake here and there is no easy way to fix inconsistencies. Supporting some existing use cases requires awkward workarounds. + +We would like there to be an alternative that provides the following properties: + +* be simple for an administrator, + +* should work even if the schema of the metadata changes in the future, e.g. new parameters are added to service levels, + +* be atomic. Group0 changes are linearizable and a backup should produce a description that corresponds to the state of metadata at a specific point—close to the time when the user issued a backup. + +# Synopsis of the solution + +Because the schema, auth, and service levels can be manipulated by administrators via the CQL interface, we want to generate a sequence of CQL statements that, when applied in order, would restore their current state. The order of those statements is important since there may be dependencies between the entities the statements would recreate, e.g. we can only grant a role to another if they already exist. If new dependencies are introduced, the CQL interface will still be usable as long as we update the implementation. + +The solution relies on the assumption that such a sequence of statements can be generated from the internal state. + +# Implementation of the solution + +We introduce a statement responsible for describing schema, auth, and service level entities: `DESC SCHEMA`. It encompasses three "tiers": + +* `DESCRIBE [FULL] SCHEMA`: describe elements of the non-system schema: keyspaces, tables, views, UDTs, etc. When `FULL` is used, it also includes the elements of the system schema, e.g. system tables. + +* `DESCRIBE [FULL] SCHEMA WITH INTERNALS`: in addition to the output of the previous tier, the statement also describes auth and service levels. The statements corresponding to restoring roles do *not* contain any information about their passwords. What's more, additional information about tables, materialized views, and secondary indices may be provided. + +* `DESCRIBE [FULL] SCHEMA WITH INTERNALS AND PASSWORDS`: aside from the information retrieved as part of the previous tier, the statements corresponding to restoring roles *do* contain information about their passwords, i.e. their salted hashes. For more information, see the relevant section below. + +Instead of `DESCRIBE`, the user can use its shortened form: `DESC`. + +As a result of the query, the user will obtain a set of rows, each of which consists of four values: + +* `keyspace_name`: the name of the keyspace the entity is part of, +* `type`: the type of the entity, +* `name`: the name of the entity, +* `create_statement`: the statement used for restoring the entity. + +All of the values are always present. + +Executing `DESCRIBE [FULL] SCHEMA WITH INTERNALS AND PASSWORDS` requires that the user performing the query be a superuser. + +## Seemingly partial information when describing auth + +It may seem as though we might miss some of the information stored by auth in some cases. To better understand the problem, +consider the following scenario: + +1. Scylla is configured to use `PasswordAuthenticator` and `CassandraAuthorizer`. That means the user can create + roles with passwords and grant them permissions. +2. The user creates new roles and grants them permissions. +3. The user decides to start using `AllowAllAuthenticator` and `AllowAllAuthorizer`. + +At this point, there are a few important things to note. + +Although the column `salted_hash` in `system.roles` is not used anymore, +its contents are still present. Similarly, even though the current authorizer doesn't need to query `system.role_permissions`, +the data that was created when `CassandraAuthorizer` was in use is still there. To summarize, it may happen that the current +configuration doesn't utilize and doesn't necessarily need all of the information that has been created. + +That phenomenon is also reflected in the semantics of various structures in auth. For instance, calling `list_all()` on +`auth::allow_all_authorizer` results in an exception instead of a set of permission grants. That example shows that implementations of the interfaces in auth can block access to data that they don't use. + +The last thing that's important to note here is the fact that since some data is not used, it's likely that new instances of that +data cannot be created either. For example, when using `AllowAllAuthenticator`, it's impossible to create a role with a password, so no new values in the column `salted_hash` in `system.roles` will appear. + +Taking all of this into consideration is crucial when designing the semantics of `DESCRIBE SCHEMA`. Should we include the information +that is unused in the output of the statement, like permission grants? Or should we not? + +The conclusion we reached is to rely on the current configuration and on it only. That means that if some data cannot be accessed +via the API, it won't be included. For example, if we use `AllowAllAuthenticator`, no salted hashes will be printed +when executing `DESCRIBE SCHEMA WITH INTERNALS AND PASSWORDS` because salted hashes are not used by the authenticator. + +The rationale for that decision is the fact that we want to backup the current state of Scylla. Restoring salted hashes, permission grants, etc. +would require changing the configuration to be even able to insert the data. That's something we want to avoid. + +For that reason, the user should be aware of this fact and make sure they are prepared for possible "data loss". + +## Restoring process and its side effects + +When a resource is created, the current role is always granted full permissions to that resource. As a consequence, the role used to restore the backup will gain permissions to all of the resources that will be recreated. + +However, we don't see it as an issue. The role that is used to restore the schema, auth, and service levels *should* be a superuser. Superusers are granted full access to all resources, so unless the superuser status of the role is revoked, there will be no unwanted side effects. + +## Restoring roles with passwords + +Scylla doesn't store the passwords of the roles; instead, it stores salted hashes corresponding to them. When the user wants to log in, they provide a password. Then, that password and so-called "salt" are used to generate a hash. Finally, that hash is compared against the salted hash stored by the database to determine if the provided password is correct. + +To restore a role with its password, we want utilize that salted hash. We introduce a new form of the `CREATE ROLE` statement: + +```sql +CREATE ROLE [ IF NOT EXISTS ] `role_name` WITH SALTED HASH = '`salted_hash`' ( AND `role_option` )* +``` + +where + +```sql +role_option: LOGIN '=' `string` + :| SUPERUSER '=' `boolean` +``` + +Performing that query will result in creating a new role whose salted hash stored in the database is exactly the same as the one provided by the user. If the specified role already exists, the query will fail and no side effect will be observed—in short, we follow the semantics of the "normal" version of `CREATE ROLE`. + +The user executing that statement is required to be a superuser. + +[^1]: See: `docs/operating-scylla/procedures/backup-restore`. diff --git a/docs/operating-scylla/procedures/backup-restore/backup.rst b/docs/operating-scylla/procedures/backup-restore/backup.rst index d1d2a440a2..8086dc4ad4 100644 --- a/docs/operating-scylla/procedures/backup-restore/backup.rst +++ b/docs/operating-scylla/procedures/backup-restore/backup.rst @@ -27,19 +27,24 @@ With time, SSTables are compacted, but the hard link keeps a copy of each file. **Procedure** -| 1. Data can only be restored from a snapshot of the table schema, where data exists in a backup. Backup your schema with the following command: +| 1. Data can only be restored from a snapshot of the table schema, where data exists in a backup. Backup your schema, roles, permissions +| and service levels with the following command: -| ``$: cqlsh -e "DESC SCHEMA WITH INTERNALS" > `` +| ``$: cqlsh -e "DESC SCHEMA WITH INTERNALS AND PASSWORDS" > `` -For example: +| For example: -| ``$: cqlsh -e "DESC SCHEMA WITH INTERNALS" > db_schema.cql`` +| ``$: cqlsh -e "DESC SCHEMA WITH INTERNALS AND PASSWORDS" > db_schema.cql`` + +| The command can only be executed by a superuser. .. warning:: - To get a proper schema description, you need to use cqlsh at least in version ``6.0.19``. Restoring a schema backup created by + To get a proper schema description, you need to use cqlsh at least in version ``6.0.23``. Restoring a schema backup created by an older version of cqlsh may lead to data resurrection or data loss. To check the version of your cqlsh, you can use ``cqlsh --version``. +See :doc:`the relevant article to learn more `. + | | 2. Take a snapshot, including every keyspace you want to backup. diff --git a/docs/operating-scylla/procedures/backup-restore/restore.rst b/docs/operating-scylla/procedures/backup-restore/restore.rst index c66f09926c..71d8b92e63 100644 --- a/docs/operating-scylla/procedures/backup-restore/restore.rst +++ b/docs/operating-scylla/procedures/backup-restore/restore.rst @@ -30,6 +30,8 @@ Procedure ``cqlsh -e "SOURCE 'centos/db_schema.cql'"`` +| **Only** a superuser should perform it. + Repeat the following steps for each node in the cluster: -------------------------------------------------------- diff --git a/replica/table.cc b/replica/table.cc index 76dfc8b027..22c48a07f7 100644 --- a/replica/table.cc +++ b/replica/table.cc @@ -2578,11 +2578,9 @@ table::seal_snapshot(sstring jsondir, std::vector file_sets) } future<> table::write_schema_as_cql(database& db, sstring dir) const { - std::ostringstream ss; + auto schema_desc = this->schema()->describe(db, cql3::describe_option::STMTS); - this->schema()->describe(db, ss, false); - - auto schema_description = ss.str(); + auto schema_description = std::move(*schema_desc.create_statement); auto schema_file_name = dir + "/schema.cql"; auto f = co_await open_checked_file_dma(general_disk_error_handler, schema_file_name, open_flags::wo | open_flags::create | open_flags::truncate); auto out = co_await make_file_output_stream(std::move(f)); diff --git a/schema/schema.cc b/schema/schema.cc index f8e1987e6a..81244938c2 100644 --- a/schema/schema.cc +++ b/schema/schema.cc @@ -8,6 +8,7 @@ #include #include +#include "cql3/description.hh" #include "db/view/view.hh" #include "timestamp.hh" #include "utils/assert.hh" @@ -783,25 +784,14 @@ static std::ostream& column_definition_as_cql_key(std::ostream& os, const column return os; } -static bool is_global_index(replica::database& db, const table_id& id, const schema& s) { +static bool is_global_index(const replica::database& db, const table_id& id, const schema& s) { return db.find_column_family(id).get_index_manager().is_global_index(s); } -static bool is_index(replica::database& db, const table_id& id, const schema& s) { +static bool is_index(const replica::database& db, const table_id& id, const schema& s) { return db.find_column_family(id).get_index_manager().is_index(s); } -sstring schema::element_type(replica::database& db) const { - if (is_view()) { - if (is_index(db, view_info()->base_id(), *this)) { - return "index"; - } else { - return "view"; - } - } - return "table"; -} - static void describe_index_columns(std::ostream& os, bool is_local, const schema& index_schema, schema_ptr base_schema) { auto index_name = secondary_index::index_name_from_table_name(index_schema.cf_name()); if (!base_schema->all_indices().contains(index_name)) { @@ -852,7 +842,9 @@ static void describe_index_columns(std::ostream& os, bool is_local, const schema os << ")"; } -std::ostream& schema::describe(replica::database& db, std::ostream& os, bool with_internals) const { +sstring schema::get_create_statement(const replica::database& db, bool with_internals) const { + std::ostringstream os; + os << "CREATE "; int n = 0; @@ -865,7 +857,8 @@ std::ostream& schema::describe(replica::database& db, std::ostream& os, bool wit describe_index_columns(os, is_local, *this, db.find_schema(view_info()->base_id())); os << ";\n"; - return os; + + return std::move(os).str(); } else { os << "MATERIALIZED VIEW " << cql3::util::maybe_quote(ks_name()) << "." << cql3::util::maybe_quote(cf_name()) << " AS\n"; os << " SELECT "; @@ -970,10 +963,30 @@ std::ostream& schema::describe(replica::database& db, std::ostream& os, bool wit } } - return os; + return std::move(os).str(); } -std::ostream& schema::schema_properties(replica::database& db, std::ostream& os) const { +cql3::description schema::describe(const replica::database& db, cql3::describe_option desc_opt) const { + const sstring type = std::invoke([&] { + if (is_view()) { + return is_index(db, view_info()->base_id(), *this) + ? "index" + : "view"; + } + return "table"; + }); + + return cql3::description { + .keyspace = ks_name(), + .type = std::move(type), + .name = cf_name(), + .create_statement = desc_opt == cql3::describe_option::NO_STMTS + ? std::nullopt + : std::make_optional(get_create_statement(db, desc_opt == cql3::describe_option::STMTS_AND_INTERNALS)) + }; +} + +std::ostream& schema::schema_properties(const replica::database& db, std::ostream& os) const { os << "bloom_filter_fp_chance = " << bloom_filter_fp_chance(); os << "\n AND caching = {"; map_as_cql_param(os, caching_options().to_map()); diff --git a/schema/schema.hh b/schema/schema.hh index 8fdb3ce9b0..bf63ff0ef2 100644 --- a/schema/schema.hh +++ b/schema/schema.hh @@ -8,6 +8,7 @@ #pragma once +#include "cql3/description.hh" #include "utils/assert.hh" #include #include @@ -32,7 +33,6 @@ #include "tombstone_gc_options.hh" #include "db/per_partition_rate_limit_options.hh" #include "schema_fwd.hh" -#include "data_dictionary/keyspace_element.hh" namespace dht { @@ -543,7 +543,7 @@ struct schema_static_props { * Not safe to access across cores because of shared_ptr's. * Use global_schema_ptr for safe across-shard access. */ -class schema final : public enable_lw_shared_from_this, public data_dictionary::keyspace_element { +class schema final : public enable_lw_shared_from_this { friend class v3_columns; public: struct dropped_column { @@ -898,11 +898,9 @@ public: // Search for an existing index with same kind and options. std::optional find_index_noname(const index_metadata& target) const; friend fmt::formatter; - virtual sstring keypace_name() const override { return ks_name(); } - virtual sstring element_name() const override { return cf_name(); } - virtual sstring element_type(replica::database& db) const override; + /*! - * \brief stream the CQL DESCRIBE output. + * \brief generate the CQL DESCRIBE output. * * The output of DESCRIBE is the CQL command to create the described table with its indexes and views. * @@ -914,11 +912,12 @@ public: * or "CREATE INDEX" depends on the type of index that schema describes (ie. Materialized View, Global * Index or Local Index). * - * When `with_internals` is true, the description is extended with table's id and dropped columns. - * The dropped columns are present in column definitions and also the `ALTER DROP` statement - * (and `ALTER ADD` if the column has been re-added) to the description. + * When `cql3::describe_option::WITH_STMTS_AND_INTERNALS` is used, the description is extended with + * table's id and dropped columns. The dropped columns are present in column definitions and also the `ALTER DROP` + * statement (and `ALTER ADD` if the column has been re-added) to the description. */ - virtual std::ostream& describe(replica::database& db, std::ostream& os, bool with_internals) const override; + cql3::description describe(const replica::database& db, cql3::describe_option) const; + // Generate ALTER TABLE/MATERIALIZED VIEW statement containing all properties with current values. // The method cannot be used on index, as indexes don't support alter statement. std::ostream& describe_alter_with_properties(replica::database& db, std::ostream& os) const; @@ -938,7 +937,9 @@ public: } private: // Print all schema properties in CQL syntax - std::ostream& schema_properties(replica::database& db, std::ostream& os) const; + std::ostream& schema_properties(const replica::database& db, std::ostream& os) const; + + sstring get_create_statement(const replica::database& db, bool with_internals) const; public: const v3_columns& v3() const { return _v3_columns; diff --git a/service/qos/service_level_controller.cc b/service/qos/service_level_controller.cc index 01cf962418..0b81099b3a 100644 --- a/service/qos/service_level_controller.cc +++ b/service/qos/service_level_controller.cc @@ -6,6 +6,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +#include "cql3/util.hh" #include "utils/assert.hh" #include #include @@ -687,6 +688,117 @@ future<> service_level_controller::unregister_subscriber(qos_configuration_chang return _subscribers.remove(subscriber); } +static sstring describe_service_level(std::string_view sl_name, const service_level_options& sl_opts) { + using slo = service_level_options; + + utils::small_vector opts{}; + + const sstring sl_name_formatted = cql3::util::maybe_quote(sl_name); + + if (auto maybe_timeout = std::get_if(&sl_opts.timeout)) { + // According to the documentation, `TIMEOUT` has to be expressed in milliseconds + // or seconds. It is therefore safe to use milliseconds here. + const auto timeout = std::chrono::duration_cast(*maybe_timeout); + opts.push_back(seastar::format("TIMEOUT = {}", timeout)); + } + + switch (sl_opts.workload) { + case slo::workload_type::batch: + opts.push_back("WORKLOAD_TYPE = 'batch'"); + break; + case slo::workload_type::interactive: + opts.push_back("WORKLOAD_TYPE = 'interactive'"); + break; + case slo::workload_type::unspecified: + break; + case slo::workload_type::delete_marker: + // `slo::workload_typ::delete_marker` is only set temporarily. When a service level + // is actually created, it never has this workload type set anymore. + on_internal_error(sl_logger, "Unexpected workload type"); + } + + if (opts.size() == 0) { + return seastar::format("CREATE SERVICE LEVEL {};", sl_name_formatted); + } + + return seastar::format("CREATE SERVICE LEVEL {} WITH {};", sl_name_formatted, fmt::join(opts, " AND ")); +} + + +future> service_level_controller::describe_created_service_levels() const { + + std::vector result{}; + + // If we use Raft, we can rely on the cache and avoid a query. + // The cache gets updated when applying Raft log, so it's always up-to-date + // with the table. + // + // If Raft is not used, that means we're performing a rolling upgrade right now + // or the migration to topology on Raft hasn't started yet. It's highly unlikely + // anyone will try to make a backup in that situation, so we don't have a branch here. + // + // If Raft is not used yet, updating the cache will happen every 10 seconds. We deem it + // good enough if someone does attempt to make a backup in that state. + for (const auto& [sl_name, sl] : _service_levels_db) { + if (sl.is_static) { + continue; + } + + result.push_back(cql3::description { + // Service levels do not belong to any keyspace. + .keyspace = std::nullopt, + .type = "service_level", + .name = sl_name, + .create_statement = describe_service_level(sl_name, sl.slo) + }); + + co_await coroutine::maybe_yield(); + } + + std::ranges::sort(result, std::less<>{}, std::mem_fn(&cql3::description::name)); + + co_return result; +} + +future> service_level_controller::describe_attached_service_levels() { + const auto attached_service_levels = co_await _auth_service.local().underlying_role_manager().query_attribute_for_all("service_level"); + + std::vector result{}; + result.reserve(attached_service_levels.size()); + + for (const auto& [role, service_level] : attached_service_levels) { + const auto formatted_role = cql3::util::maybe_quote(role); + const auto formatted_sl = cql3::util::maybe_quote(service_level); + + result.push_back(cql3::description { + // Attaching a service level doesn't belong to any keyspace. + .keyspace = std::nullopt, + .type = "service_level_attachment", + .name = service_level, + .create_statement = seastar::format("ATTACH SERVICE LEVEL {} TO {};", formatted_sl, formatted_role) + }); + + co_await coroutine::maybe_yield(); + } + + std::ranges::sort(result, std::less<>{}, [] (const cql3::description& desc) noexcept { + return std::make_tuple(std::ref(desc.name), std::ref(*desc.create_statement)); + }); + + + co_return result; +} + +future> service_level_controller::describe_service_levels() { + std::vector created_service_levels_descs = co_await describe_created_service_levels(); + std::vector attached_service_levels_descs = co_await describe_attached_service_levels(); + + created_service_levels_descs.insert(created_service_levels_descs.end(), + std::make_move_iterator(attached_service_levels_descs.begin()), std::make_move_iterator(attached_service_levels_descs.end())); + + co_return created_service_levels_descs; +} + future> get_service_level_distributed_data_accessor_for_current_version( db::system_keyspace& sys_ks, @@ -704,4 +816,4 @@ get_service_level_distributed_data_accessor_for_current_version( } } -} +} // namespace qos diff --git a/service/qos/service_level_controller.hh b/service/qos/service_level_controller.hh index f4a2898ef5..de2424f485 100644 --- a/service/qos/service_level_controller.hh +++ b/service/qos/service_level_controller.hh @@ -17,6 +17,7 @@ #include "seastarx.hh" #include "auth/role_manager.hh" #include "auth/service.hh" +#include "cql3/description.hh" #include #include "qos_common.hh" #include "service/endpoint_lifecycle_subscriber.hh" @@ -263,6 +264,8 @@ public: return _service_levels_db.contains(service_level_name); } + future> describe_service_levels(); + future<> commit_mutations(::service::group0_batch&& mc) { if (_sl_data_accessor->is_v2()) { return _sl_data_accessor->commit_mutations(std::move(mc), _global_controller_db->group0_aborter); @@ -320,6 +323,10 @@ private: }; future<> set_distributed_service_level(sstring name, service_level_options slo, set_service_level_op_type op_type, service::group0_batch& mc); + + future> describe_created_service_levels() const; + future> describe_attached_service_levels(); + public: /** diff --git a/service/storage_service.cc b/service/storage_service.cc index 1c9b3affa2..64d7c0cf9c 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -3130,12 +3130,12 @@ future<> storage_service::replicate_to_all_cores(mutable_token_metadata_ptr tmpt auto& db = ss._db.local(); auto tmptr = pending_token_metadata_ptr[this_shard_id()]; db.get_tables_metadata().for_each_table([&] (table_id id, lw_shared_ptr table) { - auto rs = db.find_keyspace(table->schema()->keypace_name()).get_replication_strategy_ptr(); + auto rs = db.find_keyspace(table->schema()->ks_name()).get_replication_strategy_ptr(); locator::effective_replication_map_ptr erm; if (auto pt_rs = rs->maybe_as_per_table()) { erm = pt_rs->make_replication_map(id, tmptr); } else { - erm = pending_effective_replication_maps[this_shard_id()][table->schema()->keypace_name()]; + erm = pending_effective_replication_maps[this_shard_id()][table->schema()->ks_name()]; } pending_table_erms[this_shard_id()].emplace(id, std::move(erm)); }); diff --git a/service/tablet_allocator.cc b/service/tablet_allocator.cc index d0cf1bf4f4..01c0bb4e81 100644 --- a/service/tablet_allocator.cc +++ b/service/tablet_allocator.cc @@ -1971,7 +1971,7 @@ public: auto tm = _db.get_shared_token_metadata().get(); lblogger.debug("Creating tablets for {}.{} id={}", s.ks_name(), s.cf_name(), s.id()); auto map = tablet_rs->allocate_tablets_for_new_table(s.shared_from_this(), tm, _config.initial_tablets_scale).get(); - muts.emplace_back(tablet_map_to_mutation(map, s.id(), s.keypace_name(), s.cf_name(), ts).get()); + muts.emplace_back(tablet_map_to_mutation(map, s.id(), s.ks_name(), s.cf_name(), ts).get()); } } diff --git a/test/boost/auth_test.cc b/test/boost/auth_test.cc index 47d110ae3e..571183d771 100644 --- a/test/boost/auth_test.cc +++ b/test/boost/auth_test.cc @@ -19,6 +19,8 @@ #include #include +#include "cql3/CqlParser.hpp" +#include "exceptions/exceptions.hh" #include "service/raft/raft_group0_client.hh" #include "test/lib/scylla_test_case.hh" #include "test/lib/cql_test_env.hh" @@ -356,3 +358,55 @@ SEASTAR_TEST_CASE(test_alter_with_workload_type) { BOOST_REQUIRE_EQUAL(e.local_client_state().get_workload_type(), service::client_state::workload_type::interactive); }, auth_on(false)); } + +SEASTAR_TEST_CASE(test_try_to_create_role_with_salted_hash_and_password) { + return do_with_cql_env_thread([] (cql_test_env& env) { + BOOST_REQUIRE_THROW( + env.execute_cql("CREATE ROLE jane WITH SALTED HASH = 'something' AND PASSWORD = 'something'").get(), + exceptions::syntax_exception); + }, auth_on(false)); +} + +SEASTAR_TEST_CASE(test_try_to_create_role_with_password_and_salted_hash) { + return do_with_cql_env_thread([] (cql_test_env& env) { + BOOST_REQUIRE_THROW( + env.execute_cql("CREATE ROLE jane WITH PASSWORD = 'something' AND SALTED HASH = 'something'").get(), + exceptions::syntax_exception); + }, auth_on(false)); +} + +SEASTAR_TEST_CASE(test_try_create_role_with_salted_hash_as_anonymous_user) { + return do_with_cql_env_thread([] (cql_test_env& env) { + env.local_client_state().set_login(auth::anonymous_user()); + env.refresh_client_state().get(); + BOOST_REQUIRE(auth::is_anonymous(*env.local_client_state().user())); + BOOST_REQUIRE_THROW(env.execute_cql("CREATE ROLE my_new_role WITH SALTED HASH = 'myhash'").get(), exceptions::unauthorized_exception); + }, auth_on(true)); +} + +SEASTAR_TEST_CASE(test_try_login_after_creating_roles_with_salted_hash) { + return do_with_cql_env_thread([] (cql_test_env& env) { + // Note: crypt(5) specifies: + // + // "Hashed passphrases are always entirely printable ASCII, and do not contain any whitespace + // or the characters `:`, `;`, `*`, `!`, or `\`. (These characters are + // used as delimiters and special markers in the passwd(5) and shadow(5) files.)" + + env.execute_cql("CREATE ROLE invalid_role WITH SALTED HASH = ';' AND LOGIN = true").get(); + env.execute_cql("CREATE ROLE valid_role WITH SALTED HASH = 'salted_hash' AND LOGIN = true").get(); + BOOST_REQUIRE_EXCEPTION(authenticate(env, "invalid_role", "pwd").get(), exceptions::authentication_exception, + exception_predicate::message_equals("Could not verify password")); + BOOST_REQUIRE_EXCEPTION(authenticate(env, "valid_role", "pwd").get(), exceptions::authentication_exception, + exception_predicate::message_equals("Username and/or password are incorrect")); + }, auth_on(true)); +} + +SEASTAR_TEST_CASE(test_try_describe_schema_with_internals_and_passwords_as_anonymous_user) { + return do_with_cql_env_thread([] (cql_test_env& env) { + env.local_client_state().set_login(auth::anonymous_user()); + env.refresh_client_state().get(); + BOOST_REQUIRE(auth::is_anonymous(*env.local_client_state().user())); + BOOST_REQUIRE_EXCEPTION(env.execute_cql("DESC SCHEMA WITH INTERNALS AND PASSWORDS").get(), exceptions::unauthorized_exception, + exception_predicate::message_equals("DESCRIBE SCHEMA WITH INTERNALS AND PASSWORDS can only be issued by a superuser")); + }, auth_on(true)); +} diff --git a/test/boost/cql_auth_syntax_test.cc b/test/boost/cql_auth_syntax_test.cc index 09b96591a4..a2d15c2bd9 100644 --- a/test/boost/cql_auth_syntax_test.cc +++ b/test/boost/cql_auth_syntax_test.cc @@ -320,3 +320,37 @@ BOOST_AUTO_TEST_CASE(grant_role) { BOOST_AUTO_TEST_CASE(revoke_role) { test_valid("REVOKE soldier FROM boromir;"); } + +BOOST_AUTO_TEST_CASE(create_role_with_salted_hash) { + test_valid("CREATE ROLE adam WITH SALTED HASH = 'something';"); +} + +BOOST_AUTO_TEST_CASE(create_role_with_salted) { + BOOST_REQUIRE_THROW( + cql3::util::do_with_parser("CREATE ROLE jane WITH SALTED = 'something';", cql3::dialect{}, std::mem_fn(&cql3_parser::CqlParser::query)), + exceptions::syntax_exception); +} + +BOOST_AUTO_TEST_CASE(create_role_with_salted_underscore_hash) { + BOOST_REQUIRE_THROW( + cql3::util::do_with_parser("CREATE ROLE jane WITH SALTED_HASH = 'something';", cql3::dialect{}, std::mem_fn(&cql3_parser::CqlParser::query)), + exceptions::syntax_exception); +} + +BOOST_AUTO_TEST_CASE(create_role_with_hash) { + BOOST_REQUIRE_THROW( + cql3::util::do_with_parser("CREATE ROLE jane WITH HASH = 'something';", cql3::dialect{}, std::mem_fn(&cql3_parser::CqlParser::query)), + exceptions::syntax_exception); +} + +BOOST_AUTO_TEST_CASE(create_role_with_salted_hash_double_quotation_marks) { + BOOST_REQUIRE_THROW( + cql3::util::do_with_parser("CREATE ROLE jane WITH SALTED HASH = \"something\";", cql3::dialect{}, std::mem_fn(&cql3_parser::CqlParser::query)), + exceptions::syntax_exception); +} + +BOOST_AUTO_TEST_CASE(create_role_with_salted_no_quotation_marks) { + BOOST_REQUIRE_THROW( + cql3::util::do_with_parser("CREATE ROLE jane WITH SALTED = something;", cql3::dialect{}, std::mem_fn(&cql3_parser::CqlParser::query)), + exceptions::syntax_exception); +} diff --git a/test/boost/cql_query_test.cc b/test/boost/cql_query_test.cc index 3e8626de6b..f338845cc5 100644 --- a/test/boost/cql_query_test.cc +++ b/test/boost/cql_query_test.cc @@ -4225,10 +4225,9 @@ SEASTAR_TEST_CASE(test_describe_simple_schema) { for (auto &&ct : cql_create_tables) { e.execute_cql(ct.second).get(); auto schema = e.local_db().find_schema("ks", ct.first); - std::ostringstream ss; + auto schema_desc = schema->describe(e.local_db(), cql3::describe_option::STMTS); - schema->describe(e.local_db(), ss, false); - BOOST_CHECK_EQUAL(normalize_white_space(ss.str()), normalize_white_space(ct.second)); + BOOST_CHECK_EQUAL(normalize_white_space(*schema_desc.create_statement), normalize_white_space(ct.second)); } }, describe_test_config()); } @@ -4293,15 +4292,14 @@ SEASTAR_TEST_CASE(test_describe_view_schema) { for (auto &&ct : cql_create_tables) { e.execute_cql(ct.second).get(); auto schema = e.local_db().find_schema("KS", ct.first); - std::ostringstream ss; + auto schema_desc = schema->describe(e.local_db(), cql3::describe_option::STMTS); - schema->describe(e.local_db(), ss, false); - BOOST_CHECK_EQUAL(normalize_white_space(ss.str()), normalize_white_space(ct.second)); + BOOST_CHECK_EQUAL(normalize_white_space(*schema_desc.create_statement), normalize_white_space(ct.second)); auto base_schema = e.local_db().find_schema("KS", "cF"); - std::ostringstream base_ss; - base_schema->describe(e.local_db(), base_ss, false); - BOOST_CHECK_EQUAL(normalize_white_space(base_ss.str()), normalize_white_space(base_table)); + auto base_schema_desc = base_schema->describe(e.local_db(), cql3::describe_option::STMTS); + + BOOST_CHECK_EQUAL(normalize_white_space(*base_schema_desc.create_statement), normalize_white_space(base_table)); } }, describe_test_config()); } diff --git a/test/boost/user_types_test.cc b/test/boost/user_types_test.cc index 802fcb026a..cf63658af7 100644 --- a/test/boost/user_types_test.cc +++ b/test/boost/user_types_test.cc @@ -11,6 +11,8 @@ #include "test/lib/cql_test_env.hh" #include "test/lib/cql_assertions.hh" +#include "types/map.hh" +#include "types/set.hh" #include "types/user.hh" #include "types/list.hh" #include "test/lib/exception_utils.hh" @@ -640,3 +642,32 @@ SEASTAR_TEST_CASE(test_user_type_quoted) { // Pass if the above CREATE TABLE completes without an exception. }); } + +SEASTAR_TEST_CASE(test_cql3_name_without_frozen) { + return do_with_cql_env_thread([] (cql_test_env& e) { + const sstring type_name = "ut1"; + const sstring frozen_type_name = seastar::format("frozen<{}>", type_name); + + const auto type_ptr = user_type_impl::get_instance("ks", to_bytes(type_name), + {to_bytes("my_int")}, {int32_type}, false); + BOOST_REQUIRE(type_ptr->cql3_type_name_without_frozen() == type_name); + + const auto wrapped_type_ptr = user_type_impl::get_instance("ks", to_bytes("wrapped_type"), + {to_bytes("field_name")}, {type_ptr->freeze()}, false); + const auto& field_type = wrapped_type_ptr->field_types()[0]; + BOOST_REQUIRE(field_type->cql3_type_name() == frozen_type_name); + BOOST_REQUIRE(field_type->cql3_type_name_without_frozen() == type_name); + + const sstring set_type_name = seastar::format("set<{}>", frozen_type_name); + const auto set_type_ptr = set_type_impl::get_instance(type_ptr->freeze(), false); + BOOST_REQUIRE(set_type_ptr->cql3_type_name_without_frozen() == set_type_name); + + const sstring map_type_name = seastar::format("map", frozen_type_name); + const auto map_type_ptr = map_type_impl::get_instance(int32_type, type_ptr->freeze(), false); + BOOST_REQUIRE(map_type_ptr->cql3_type_name_without_frozen() == map_type_name); + + const sstring list_type_name = seastar::format("list<{}>", frozen_type_name); + const auto list_type_ptr = list_type_impl::get_instance(type_ptr->freeze(), false); + BOOST_REQUIRE(list_type_ptr->cql3_type_name_without_frozen() == list_type_name); + }); +} diff --git a/test/cql-pytest/test_describe.py b/test/cql-pytest/test_describe.py index 65c7d36023..0cc504696f 100644 --- a/test/cql-pytest/test_describe.py +++ b/test/cql-pytest/test_describe.py @@ -9,11 +9,46 @@ import pytest import random -from pytest import fixture -from contextlib import contextmanager, ExitStack -from util import new_type, unique_name, new_test_table, new_test_keyspace, new_function, new_aggregate, new_cql, keyspace_has_tablets, unique_name_prefix -from cassandra.protocol import InvalidRequest import re +from contextlib import contextmanager, ExitStack +from util import new_type, unique_name, new_test_table, new_test_keyspace, new_function, new_aggregate, new_cql, keyspace_has_tablets, unique_name_prefix, new_user, new_session +from cassandra.protocol import InvalidRequest, Unauthorized +from collections.abc import Iterable +from typing import Any + +# Type of the row returned by `DESC` statements. It's of form +# (keyspace_name, type, name, create_statement) +DescRowType = Any + +DEFAULT_SUPERUSER = "cassandra" + +def filter_non_default_user(desc_result_iter: Iterable[DescRowType]) -> Iterable[DescRowType]: + return filter(lambda result: result.name != DEFAULT_SUPERUSER, desc_result_iter) + +### + +def filter_roles(desc_result_iter: Iterable[DescRowType]) -> Iterable[DescRowType]: + return filter(lambda result: result.type == "role", desc_result_iter) + +def filter_grant_roles(desc_result_iter: Iterable[DescRowType]) -> Iterable[DescRowType]: + return filter(lambda result: result.type == "grant_role", desc_result_iter) + +def filter_grant_permissions(desc_result_iter: Iterable[DescRowType]) -> Iterable[DescRowType]: + return filter(lambda result: result.type == "grant_permission", desc_result_iter) + +def filter_service_levels(desc_result_iter: Iterable[DescRowType]) -> Iterable[DescRowType]: + return filter(lambda result: result.type == "service_level", desc_result_iter) + +def filter_attached_service_levels(desc_result_iter: Iterable[DescRowType]) -> Iterable[DescRowType]: + return filter(lambda result: result.type == "service_level_attachment", desc_result_iter) + +### + +def extract_names(desc_result_iter: Iterable[DescRowType]) -> Iterable[str]: + return map(lambda result: result.name, desc_result_iter) + +def extract_create_statements(desc_result_iter: Iterable[DescRowType]) -> Iterable[str]: + return map(lambda result: result.create_statement, desc_result_iter) # (`element` refers to keyspace or keyspace's element(table, type, function, aggregate)) # There are 2 main types of tests: @@ -1193,3 +1228,1342 @@ def new_random_type(cql, keyspace, udts=[]): return new_type(cql, keyspace, f"({fields})") ### =========================================================================== + +################################################################################ +# ............................................................................ # +# ------------------------------- DESCRIPTION -------------------------------- # +# ............................................................................ # +# ============================================================================ # +# # +# The tests below correspond to the task `scylladb/scylladb#18750`: # +# "auth on raft: safe backup and restore" # +# # +# We want to test the following features related to the issue: # +# # +# 1. Creating roles when providing `SALTED HASH`, # +# 2. The behavior of `DESC SCHEMA WITH INTERNALS (AND PASSWORDS)` and # +# the correctness of statements that it produces and which are related # +# to the referenced issue: auth and service levels. # +# # +# To do that, we use the following pattern for most of the cases we should # +# cover in this file: # +# # +# 1. Format of the returned result by the query: # +# - Are all of the values in the columns present and correct? # +# 2. Formatting of identifiers with quotation marks. # +# 3. Formatting of identifiers with uppercase characters. # +# 4. Formatting of identifiers with unicode characters. # +# 5. Test(s) verifying that all of the cases are handled and `DESC SCHEMA` # +# prints them properly. # +# # +################################################################################ + +################################################################################ +# ............................................................................ # +# ---------------------------------- NOTES ----------------------------------- # +# ............................................................................ # +# ============================================================================ # +# # +# 1. Every create statement corresponding to auth and service levels returned # +# by # +# `DESC SCHEMA WITH INTERNALS (AND PASSWORDS)` # +# is termined with a semicolon. # +# # +# 2. `CREATE ROLE` statements always preserve the following order of options: # +# `SALTED HASH`, `LOGIN`, `SUPERUSER` # +# Aside from `SALTED HASH`, which only appears when executing # +# `DESC SCHEMA WITH INTERNALS AND PASSWORDS` # +# the parameters are always present. # +# # +# 3. *ALL* create statements returned by # +# `DESC SCHEMA WITH INTERNALS (AND PASSWORDS)` # +# and related to AUTH/service levels use capital letters for CQL syntax. # +# # +# 4. If an identifier needs to be quoted, e.g. because it contains whitespace # +# characters, it will be wrapped with double quoatation marks. # +# There are three exceptions to that rule: # +# # +# (i) the `WORKLOAD_TYPE` option when creating a service level, # +# (ii) the `PASSWORD` option when creating a role, # +# (iii) the `SALTED HASH` option when creating a role. # +# # +# The exceptions are enforced by the CQL grammar used in Scylla. # +# # +# 5. Statements for creating service levels always have options listed in the # +# following order: # +# `TIMEOUT`, `WORKLOAD_TYPE`, `SHARES` # +# If an option is unnecessary (e.g. there's no timeout or the workload # +# type is unspecified -- default!), it's not present in the create # +# statement of the result of `DESC SCHEMA WITH INTERNALS`. # +# # +# 6. The `TIMEOUT` option in `CREATE SERVICE LEVEL` statements returned # +# by `DESC SCHEMA WITH INTERNALS` always uses milliseconds as its # +# resolution. # +# # +# 7. We create test keyspaces manually here. The rationale for that is # +# the fact that the creator of a resource automatically obtains all # +# permissions on the resource. Since in these tests we verify permission # +# grants, we want to have full control over who creates what. # +# # +################################################################################ + +def sanitize_identifier(identifier: str, quotation_mark: str) -> str: + doubled_quotation_mark = quotation_mark + quotation_mark + return identifier.replace(quotation_mark, doubled_quotation_mark) + +def sanitize_password(password: str) -> str: + return sanitize_identifier(password, "'") + +def make_identifier(identifier: str, quotation_mark: str) -> str: + return quotation_mark + sanitize_identifier(identifier, quotation_mark) + quotation_mark + +### + +KS_AND_TABLE_PERMISSIONS = ["CREATE", "ALTER", "DROP", "MODIFY", "SELECT", "AUTHORIZE"] + +### + +class AuthSLContext: + def __init__(self, cql, ks=None): + self.cql = cql + self.ks = ks + + def __enter__(self): + if self.ks: + self.cql.execute(f"CREATE KEYSPACE {self.ks} WITH REPLICATION = {{ 'class': 'SimpleStrategy', 'replication_factor': 1 }}") + return self + + def __exit__(self, exc_type, exc_val, exc_tb) -> None: + if self.ks: + self.cql.execute(f"DROP KEYSPACE {self.ks}") + + roles_iter = self.cql.execute(f"SELECT role FROM system.roles") + roles_iter = filter(lambda record: record.role != DEFAULT_SUPERUSER, roles_iter) + roles = [record.role for record in roles_iter] + for role in roles: + self.cql.execute(f"DROP ROLE {make_identifier(role, quotation_mark='"')}") + + service_levels_iter = self.cql.execute("LIST ALL SERVICE LEVELS") + service_levels = [record.service_level for record in service_levels_iter] + for sl in service_levels: + self.cql.execute(f"DROP SERVICE LEVEL {make_identifier(sl, quotation_mark='"')}") + +### + +def test_create_role_with_salted_hash(cql): + """ + Verify that creating a role with a salted hash works correctly, i.e. that the salted hash + present in `system.roles` is the same as the one we provide. + """ + + with AuthSLContext(cql): + role = "andrew" + # Arbitrary salted hash. Could be anything. + # We don't use characters that won't be generated, i.e.: + # `:`, `;`, `*`, `!`, and `\`, + # but Scylla should technically accept them too. + salted_hash = "@#$%^&()`,./{}[]abcdefghijklmnopqrstuwvxyzABCDEFGHIJKLMNOPQRSTUWVXYZ123456789~-_=+|" + cql.execute(f"CREATE ROLE {role} WITH SALTED HASH = '{sanitize_password(salted_hash)}'") + + [result] = cql.execute(f"SELECT salted_hash FROM system.roles WHERE role = '{role}'") + assert salted_hash == result.salted_hash + + +def test_create_role_with_salted_hash_authorization(cql): + """ + Verify that roles that aren't superusers cannot perform `CREATE ROLE WITH SALTED HASH`. + """ + + with AuthSLContext(cql): + def try_create_role_with_salted_hash(role): + with new_session(cql, role) as ncql: + with pytest.raises(Unauthorized): + ncql.execute("CREATE ROLE some_unused_name WITH SALTED HASH = 'somesaltedhash'") + + # List of form (role name, list of permission grants to the role) + r1 = "andrew" + r2 = "jane" + + with new_user(cql, r1), new_user(cql, r2): + # This also grants access to system tables. + cql.execute(f"GRANT ALL ON ALL KEYSPACES TO {r2}") + + try_create_role_with_salted_hash(r1) + try_create_role_with_salted_hash(r2) + + r3 = "bob" + + with new_user(cql, r3, with_superuser_privileges=True): + with new_session(cql, r3) as ncql: + ncql.execute("CREATE ROLE some_unused_name WITH SALTED HASH = 'somesaltedhash'") + +### + +def test_desc_authorization(cql): + """ + Verify that Scylla rejects performing `DESC SCHEMA WITH INTERNALS AND PASSWORDS` if the user + sending the request is not a superuser, even if they have all permissions to relevant system tables. + """ + + with AuthSLContext(cql): + def try_describe_with_passwords(role): + with new_session(cql, role) as ncql: + with pytest.raises(Unauthorized): + ncql.execute("DESCRIBE SCHEMA WITH INTERNALS AND PASSWORDS") + + # List of form (role name, list of permission grants to the role) + r1 = "andrew" + r2 = "jane" + + with new_user(cql, r1), new_user(cql, r2): + # This also grants access to system tables. + cql.execute(f"GRANT ALL ON ALL KEYSPACES TO {r2}") + + try_describe_with_passwords(r1) + try_describe_with_passwords(r2) + +def test_desc_roles_format(cql): + """ + Verify that the format of the output of `DESC SCHEMA WITH INTERNALS` corresponding to + creating roles is of the expected form. + """ + + with AuthSLContext(cql): + role_name = "andrew" + stmt = f"CREATE ROLE {role_name} WITH LOGIN = false AND SUPERUSER = false;" + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_roles(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + [result] = list(desc_iter) + + assert result.keyspace_name == None + assert result.type == "role" + assert result.name == role_name + assert result.create_statement == stmt + +def test_desc_roles_quotation_marks(cql): + """ + Verify that statements corresponding to creating roles correctly format quotation marks. + """ + + with AuthSLContext(cql): + andrew_raw = "andrew \" 'the great'" + jane_raw = "jane ' \"the wise\"" + + andrew_salted_hash_raw = "my \" 'salted hash'" + jane_salted_hash_raw = "my ' \"other salted hash\"" + + andrew_single_quote = make_identifier(andrew_raw, quotation_mark="'") + andrew_double_quote = make_identifier(andrew_raw, quotation_mark='"') + jane_double_quote = make_identifier(jane_raw, quotation_mark='"') + + andrew_salted_hash = make_identifier(andrew_salted_hash_raw, quotation_mark="'") + jane_salted_hash = make_identifier(jane_salted_hash_raw, quotation_mark="'") + + cql.execute(f"CREATE ROLE {andrew_single_quote} WITH SALTED HASH = {andrew_salted_hash}") + cql.execute(f"CREATE ROLE {jane_double_quote} WITH SALTED HASH = {jane_salted_hash}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS AND PASSWORDS") + desc_iter = filter_roles(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + element_names_iter = extract_names(desc_elements) + + assert set(element_names_iter) == {andrew_double_quote, jane_double_quote} + + desc_iter = extract_create_statements(desc_elements) + + expected_result = { + f"CREATE ROLE {andrew_double_quote} WITH SALTED HASH = {andrew_salted_hash} AND LOGIN = false AND SUPERUSER = false;", + f"CREATE ROLE {jane_double_quote} WITH SALTED HASH = {jane_salted_hash} AND LOGIN = false AND SUPERUSER = false;" + } + + assert set(desc_iter) == expected_result + +def test_desc_roles_uppercase(cql): + """ + Verify that statements corresponding to creating roles correctly format uppercase characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + role = '"myRole"' + cql.execute(f"CREATE ROLE {role}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_roles(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert list(role_iter) == [role] + + desc_iter = extract_create_statements(desc_elements) + + assert list(desc_iter) == [f"CREATE ROLE {role} WITH LOGIN = false AND SUPERUSER = false;"] + +def test_desc_roles_unicode(cql): + """ + Verify that statements to creating roles can contain unicode characters. + """ + + with AuthSLContext(cql): + role = '"ユーザー"' + cql.execute(f"CREATE ROLE {role}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_roles(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert list(role_iter) == [role] + + desc_iter = extract_create_statements(desc_elements) + + assert list(desc_iter) == [f"CREATE ROLE {role} WITH LOGIN = false AND SUPERUSER = false;"] + +def test_desc_roles(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to creating roles + is as expected for various different cases. + """ + + with AuthSLContext(cql): + roles = ["andrew", "alice", '"m!ch@el j4cks0n"'] + roles_password = ["fred", "julie", '"hi, I like sunsets"'] + roles_can_login = ["bob", "jane", '"very weird nam3 Fu!! of character$"'] + roles_superuser = ["gustang", "devon", '"my h4ppy c0mp@n!0n!"'] + roles_can_login_and_superuser = ["peter", "susan", '"the k!ng 0f 3vryth!ng th4t 3x!$t$ @"'] + + for idx, role in enumerate(roles): + cql.execute(f"CREATE ROLE {role}") + for idx, role in enumerate(roles_password): + cql.execute(f"CREATE ROLE {role} WITH PASSWORD = 'my_password{idx}'") + for role in roles_can_login: + cql.execute(f"CREATE ROLE {role} WITH LOGIN = true") + for role in roles_superuser: + cql.execute(f"CREATE ROLE {role} WITH SUPERUSER = true") + for role in roles_can_login_and_superuser: + cql.execute(f"CREATE ROLE {role} WITH SUPERUSER = true AND LOGIN = true") + + create_role_stmts = [ + [f"CREATE ROLE {role} WITH LOGIN = false AND SUPERUSER = false;" for role in roles], + [f"CREATE ROLE {role} WITH LOGIN = false AND SUPERUSER = false;" for role in roles_password], + [f"CREATE ROLE {role} WITH LOGIN = true AND SUPERUSER = false;" for role in roles_can_login], + [f"CREATE ROLE {role} WITH LOGIN = false AND SUPERUSER = true;" for role in roles_superuser], + [f"CREATE ROLE {role} WITH LOGIN = true AND SUPERUSER = true;" for role in roles_can_login_and_superuser], + [f"CREATE ROLE IF NOT EXISTS {DEFAULT_SUPERUSER} WITH LOGIN = true AND SUPERUSER = true;"] + ] + # Flatten the list of lists to a list. + create_role_stmts = sum(create_role_stmts, []) + create_role_stmts = set(create_role_stmts) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_roles(desc_iter) + desc_create_role_stmts = set(extract_create_statements(desc_iter)) + + assert create_role_stmts == desc_create_role_stmts + +def test_desc_roles_with_passwords(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS AND PASSWORDS` corresponding to creating roles + is as expected for various different cases. + """ + + with AuthSLContext(cql): + role_without_pass = "bob" + role_with_pass = "alice" + + create_stmt_without_pass = f"CREATE ROLE {role_without_pass} WITH LOGIN = false AND SUPERUSER = false;" + create_stmt_with_pass = f"CREATE ROLE {role_with_pass} WITH PASSWORD = 'some_funky_password'" + + cql.execute(create_stmt_without_pass) + cql.execute(create_stmt_with_pass) + + [salted_hash_result] = cql.execute(f"SELECT salted_hash FROM system.roles WHERE role = '{role_with_pass}'") + salted_hash = salted_hash_result.salted_hash + create_stmt_with_salted_hash = f"CREATE ROLE {role_with_pass} WITH SALTED HASH = '{sanitize_password(salted_hash)}' AND LOGIN = false AND SUPERUSER = false;" + + stmts = [create_stmt_without_pass, create_stmt_with_salted_hash] + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS AND PASSWORDS") + desc_iter = filter_roles(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + desc_iter = extract_create_statements(desc_iter) + + assert set(stmts) == set(desc_iter) + +def test_desc_role_grants_format(cql): + """ + Verify that the format of the output of `DESC SCHEMA WITH INTERNALS` corresponding to + granting roles is of the expected form. + """ + + with AuthSLContext(cql): + [r1, r2] = ["andrew", "jane"] + + cql.execute(f"CREATE ROLE {r1}") + cql.execute(f"CREATE ROLE {r2}") + + stmt = f"GRANT {r1} TO {r2};" + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_roles(desc_iter) + + [result] = list(desc_iter) + + assert result.keyspace_name == None + assert result.type == "grant_role" + assert result.name == r1 + assert result.create_statement == stmt + +def test_desc_role_grants_quotation_marks(cql): + """ + Verify that statements corresponding to granting roles correctly format quotation marks. + """ + + with AuthSLContext(cql): + andrew_raw = "andrew \" 'the great'" + jane_raw = "jane ' \"the wise\"" + + andrew_single_quote = make_identifier(andrew_raw, quotation_mark="'") + andrew_double_quote = make_identifier(andrew_raw, quotation_mark='"') + jane_double_quote = make_identifier(jane_raw, quotation_mark='"') + + cql.execute(f"CREATE ROLE {andrew_single_quote}") + cql.execute(f"CREATE ROLE {jane_double_quote}") + + cql.execute(f"GRANT {andrew_single_quote} TO {jane_double_quote}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_roles(desc_iter) + + desc_elements = [*desc_iter] + element_names_iter = extract_names(desc_elements) + + assert set(element_names_iter) == {andrew_double_quote} + + desc_iter = extract_create_statements(desc_elements) + + expected_result = f"GRANT {andrew_double_quote} TO {jane_double_quote};" + assert [expected_result] == list(desc_iter) + +def test_desc_role_grants_uppercase(cql): + """ + Verify that statements corresponding to granting roles correctly format uppercase characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + r1 = '"myRole"' + r2 = '"otherRole"' + + cql.execute(f"CREATE ROLE {r1}") + cql.execute(f"CREATE ROLE {r2}") + cql.execute(f"GRANT {r1} TO {r2}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_roles(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert list(role_iter) == [r1] + + desc_iter = extract_create_statements(desc_elements) + + assert list(desc_iter) == [f"GRANT {r1} TO {r2};"] + +def test_desc_role_grants_unicode(cql): + """ + Verify that statements corresponding to granting roles correctly format unicode characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + r1 = '"ユーザー"' + r2 = '"私の猫"' + + cql.execute(f"CREATE ROLE {r1}") + cql.execute(f"CREATE ROLE {r2}") + cql.execute(f"GRANT {r1} TO {r2}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_roles(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert list(role_iter) == [r1] + + desc_iter = extract_create_statements(desc_elements) + + assert list(desc_iter) == [f"GRANT {r1} TO {r2};"] + +def test_desc_role_grants(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to granting roles + is as expected for various different cases. + """ + + with AuthSLContext(cql): + [r1, r2, r3, r4] = ["andrew", '"b0b, my f@vor!t3 fr!3nd :)"', "jessica", "kate"] + # List whose each element is a pair of form: + # (role, list of granted roles) + roles = [ + (r1, []), + (r2, [r1]), + (r3, [r2]), + (r4, [r1, r3]) + ] + + for role, _ in roles: + cql.execute(f"CREATE ROLE {role}") + for role, grants in roles: + for grant in grants: + cql.execute(f"GRANT {grant} TO {role}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_roles(desc_iter) + desc_grants = list(extract_create_statements(desc_iter)) + + expected_grants = [[f"GRANT {grant} TO {role};" for grant in grants] for role, grants in roles] + # Flatten the list of lists to a list. + expected_grants = sum(expected_grants, []) + + assert set(expected_grants) == set(desc_grants) + +def test_desc_grant_permission_format(cql): + """ + Verify that the format of the output of `DESC SCHEMA WITH INTERNALS` corresponding to + granting permissions is of the expected form. + """ + + with AuthSLContext(cql): + role_name = "kate" + cql.execute(f"CREATE ROLE {role_name}") + + stmt = f"GRANT SELECT ON ALL KEYSPACES TO {role_name};" + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + # We need to fliter out the default superuser because when it creates a role, + # it automatically obtains all of permissions to manipulate it. + desc_iter = filter_non_default_user(desc_iter) + + [result] = list(desc_iter) + + assert result.keyspace_name == None + assert result.type == "grant_permission" + assert result.name == role_name + assert result.create_statement == stmt + +def test_desc_grant_permission_quotation_marks(cql): + """ + Verify that statements corresponding to granting permissions correctly format quotation marks. + """ + + with AuthSLContext(cql): + andrew_raw = "andrew \" 'the great'" + jane_raw = "jane ' \"the wise\"" + + andrew_single_quote = make_identifier(andrew_raw, quotation_mark="'") + andrew_double_quote = make_identifier(andrew_raw, quotation_mark='"') + jane_double_quote = make_identifier(jane_raw, quotation_mark='"') + + cql.execute(f"CREATE ROLE {andrew_single_quote}") + cql.execute(f"CREATE ROLE {jane_double_quote}") + + cql.execute(f"GRANT SELECT ON ALL KEYSPACES TO {andrew_single_quote}") + cql.execute(f"GRANT ALTER ON ALL KEYSPACES TO {jane_double_quote}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + element_names_iter = extract_names(desc_elements) + + assert set(element_names_iter) == {andrew_double_quote, jane_double_quote} + + desc_iter = extract_create_statements(desc_elements) + + expected_result = { + f"GRANT SELECT ON ALL KEYSPACES TO {andrew_double_quote};", + f"GRANT ALTER ON ALL KEYSPACES TO {jane_double_quote};" + } + + assert set(desc_iter) == expected_result + +def test_desc_auth_different_permissions(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to granting permissions + is as expected for various different cases. Here we test different kinds of permissions specifically. + """ + + with AuthSLContext(cql, ks="my_ks") as ctx: + all_ks_grants = [f"GRANT {permission} ON ALL KEYSPACES TO {{}};" for permission in KS_AND_TABLE_PERMISSIONS] + specific_ks_grants = [f"GRANT {permission} ON KEYSPACE {ctx.ks} TO {{}};" for permission in KS_AND_TABLE_PERMISSIONS] + + grants = [*all_ks_grants, *specific_ks_grants, r"GRANT DESCRIBE ON ALL ROLES TO {};"] + + roles = [f"my_role_{idx}" for idx in range(len(grants))] + grants = [grant.format(roles[idx]) for idx, grant in enumerate(grants)] + + for role in roles: + cql.execute(f"CREATE ROLE {role}") + + for grant in grants: + cql.execute(grant) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + # Creating a resource automatically grants all permissioners on it to the creator. + # That's why we need to filter them out here. Normally, we could check them too, + # but it's unpredictable how many keyspaces there are when this test case is being + # executed, and we'd need to take them into consideration too. So we skip them. + desc_iter = filter_non_default_user(desc_iter) + desc_iter = extract_create_statements(desc_iter) + + assert set(grants) == set(desc_iter) + +def test_desc_data_permissions_uppercase(cql): + """ + Verify that statements corresponding to granting data permissions correctly format uppercase characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + ks = '"myKs"' + with AuthSLContext(cql, ks=ks): + r1 = '"myRole"' + r2 = '"someOtherRole"' + r3 = '"YetANOTHERrole"' + + roles = {r1, r2, r3} + + for role in roles: + cql.execute(f"CREATE ROLE {role}") + + table = '"myTable"' + cql.execute(f"CREATE TABLE {ks}.{table} (pk int PRIMARY KEY, t int)") + + stmts = { + f"GRANT SELECT ON ALL KEYSPACES TO {r1};", + f"GRANT SELECT ON KEYSPACE {ks} TO {r2};", + f"GRANT SELECT ON {ks}.{table} TO {r3};" + } + + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + desc_elements = list(desc_iter) + + roles_iter = map(lambda row: row.name, desc_elements) + assert set(roles_iter) == roles + + desc_iter = extract_create_statements(desc_elements) + assert set(desc_iter) == stmts + +def test_desc_data_permissions_unicode(cql): + """ + Verify that statements corresponding to granting permissions to data resources correctly format + unicode characters, i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql, ks="my_ks") as ctx: + r1 = '"ユーザー"' + r2 = '"私の猫"' + r3 = '"山羊"' + + roles = {r1, r2, r3} + + for role in roles: + cql.execute(f"CREATE ROLE {role}") + + table = "my_table" + cql.execute(f"CREATE TABLE {ctx.ks}.{table} (pk int PRIMARY KEY, t int)") + + stmts = { + f"GRANT SELECT ON ALL KEYSPACES TO {r1};", + f"GRANT SELECT ON KEYSPACE {ctx.ks} TO {r2};", + f"GRANT SELECT ON {ctx.ks}.{table} TO {r3};" + } + + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + desc_elements = list(desc_iter) + + roles_iter = map(lambda row: row.name, desc_elements) + assert set(roles_iter) == roles + + desc_iter = extract_create_statements(desc_elements) + assert set(desc_iter) == stmts + +def test_desc_data_permissions(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to granting permissions + is as expected for various different cases. Here we test data resources specifically. + """ + + with AuthSLContext(cql, ks="my_ks") as ctx: + all_ks_role = "mary" + spec_ks_role = '"r0bb, my gr3@t3st fr!3nd :)"' + spec_table_role = "scarlet" + + for role in [all_ks_role, spec_ks_role, spec_table_role]: + cql.execute(f"CREATE ROLE {role}") + + table_name = "my_table" + # Note: When the keyspace `ctx.ks` is dropped, the table will be removed as well. + # That's why there's no need to clean up this table later. + cql.execute(f"CREATE TABLE {ctx.ks}.{table_name} (a int PRIMARY KEY, b int)") + + all_ks_stmt = f"GRANT CREATE ON ALL KEYSPACES TO {all_ks_role};" + spec_ks_stmt = f"GRANT ALTER ON KEYSPACE {ctx.ks} TO {spec_ks_role};" + spec_table_stmt = f"GRANT MODIFY ON {ctx.ks}.{table_name} TO {spec_table_role};" + + stmts = [all_ks_stmt, spec_ks_stmt, spec_table_stmt] + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + desc_iter = extract_create_statements(desc_iter) + + assert set(stmts) == set(desc_iter) + +def test_desc_role_permissions_uppercase(cql): + """ + Verify that statements corresponding to granting role permissions correctly format uppercase characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + r1 = '"myRole"' + r2 = '"MyOtherRole"' + + roles = {r1, r2} + for role in roles: + cql.execute(f"CREATE ROLE {role}") + + stmts = { + f"GRANT AUTHORIZE ON ALL ROLES TO {r1};", + f"GRANT ALTER ON ROLE {r1} TO {r2};" + } + + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert set(role_iter) == roles + + desc_iter = extract_create_statements(desc_elements) + assert set(desc_iter) == stmts + +def test_desc_role_permissions_unicode(cql): + """ + Verify that statements corresponding to granting permissions to role resources correctly format + unicode characters, i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + r1 = '"ユーザー"' + r2 = '"私の猫"' + + roles = {r1, r2} + for role in roles: + cql.execute(f"CREATE ROLE {role}") + + stmts = { + f"GRANT AUTHORIZE ON ALL ROLES TO {r1};", + f"GRANT ALTER ON ROLE {r1} TO {r2};" + } + + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert set(role_iter) == roles + + desc_iter = extract_create_statements(desc_elements) + assert set(desc_iter) == stmts + +def test_desc_role_permissions(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to granting permissions + is as expected for various different cases. Here we test role permissions specifically. + """ + + with AuthSLContext(cql): + all_roles_role = "howard" + specific_role_role = '"h! th3r3 str@ng3r :)"' + + for role in [all_roles_role, specific_role_role]: + cql.execute(f"CREATE ROLE {role}") + + all_roles_stmt = f"GRANT AUTHORIZE ON ALL ROLES TO {all_roles_role};" + specific_role_stmt = f"GRANT ALTER ON ROLE {all_roles_role} TO {specific_role_role};" + + stmts = [all_roles_stmt, specific_role_stmt] + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + desc_iter = extract_create_statements(desc_iter) + + assert set(stmts) == set(desc_iter) + +def test_desc_udf_permissions_uppercase(cql): + """ + Verify that statements corresponding to granting permissions to UDFs correctly format uppercase characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + ks = '"myKs"' + with AuthSLContext(cql, ks=ks): + all_funcs_role = '"myRole"' + all_funcs_in_ks_role = '"MyOtherRole"' + specific_func_role = '"ROLE"' + + for role in [all_funcs_role, all_funcs_in_ks_role, specific_func_role]: + cql.execute(f"CREATE ROLE {role}") + + func_name = '"functionName"' + type_name = '"TypeName"' + + cql.execute(f"CREATE TYPE {ks}.{type_name} (value int)") + cql.execute(f"""CREATE FUNCTION {ks}.{func_name}(val1 int, val2 {type_name}) + RETURNS NULL ON NULL INPUT + RETURNS int + LANGUAGE lua + AS $$ return val1 + val2.value $$""") + + stmts = { + f"GRANT ALTER ON ALL FUNCTIONS TO {all_funcs_role};", + f"GRANT EXECUTE ON ALL FUNCTIONS IN KEYSPACE {ks} TO {all_funcs_in_ks_role};", + f"GRANT DROP ON FUNCTION {ks}.{func_name}(int, {type_name}) TO {specific_func_role};" + } + + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert set(role_iter) == {all_funcs_role, all_funcs_in_ks_role, specific_func_role} + + desc_iter = extract_create_statements(desc_elements) + assert set(desc_iter) == stmts + +def test_desc_udf_permissions_unicode(cql): + """ + Verify that statements corresponding to granting permissions to UDFs correctly format + unicode characters, i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql, ks="my_ks") as ctx: + all_funcs_role = '"ユーザー"' + all_funcs_in_ks_role = '"私の猫"' + specific_func_role = '"山羊"' + + for role in [all_funcs_role, all_funcs_in_ks_role, specific_func_role]: + cql.execute(f"CREATE ROLE {role}") + + func_name = '"関数"' + type_name = '"データ型"' + + cql.execute(f"CREATE TYPE {ctx.ks}.{type_name} (value int)") + cql.execute(f"""CREATE FUNCTION {ctx.ks}.{func_name}(val1 int, val2 {type_name}) + RETURNS NULL ON NULL INPUT + RETURNS int + LANGUAGE lua + AS $$ return val1 + val2.value $$""") + + stmts = { + f"GRANT ALTER ON ALL FUNCTIONS TO {all_funcs_role};", + f"GRANT EXECUTE ON ALL FUNCTIONS IN KEYSPACE {ctx.ks} TO {all_funcs_in_ks_role};", + f"GRANT DROP ON FUNCTION {ctx.ks}.{func_name}(int, {type_name}) TO {specific_func_role};" + } + + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert set(role_iter) == {all_funcs_role, all_funcs_in_ks_role, specific_func_role} + + desc_iter = extract_create_statements(desc_elements) + assert set(desc_iter) == stmts + +def test_desc_udf_permissions(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to granting permissions + is as expected for various different cases. Here we test UDFs specifically. + """ + + with AuthSLContext(cql, ks="my_ks") as ctx: + all_funcs_role = "miley" + all_funcs_in_ks_role = '"v3ry w3!rd n@m3!"' + specific_func_role = "ricard" + + for role in [all_funcs_role, all_funcs_in_ks_role, specific_func_role]: + cql.execute(f"CREATE ROLE {role}") + + func_name = '"funct!0n n@m3"' + type_name = '"qu!t3 int3r3st!ng typ3 :)"' + + cql.execute(f"CREATE TYPE {ctx.ks}.{type_name} (value int)") + cql.execute(f"""CREATE FUNCTION {ctx.ks}.{func_name}(val1 int, val2 {type_name}) + RETURNS NULL ON NULL INPUT + RETURNS int + LANGUAGE lua + AS $$ return val1 + val2.value $$""") + + stmts = { + f"GRANT ALTER ON ALL FUNCTIONS TO {all_funcs_role};", + f"GRANT EXECUTE ON ALL FUNCTIONS IN KEYSPACE {ctx.ks} TO {all_funcs_in_ks_role};", + f"GRANT DROP ON FUNCTION {ctx.ks}.{func_name}(int, {type_name}) TO {specific_func_role};" + } + + for stmt in stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_grant_permissions(desc_iter) + desc_iter = filter_non_default_user(desc_iter) + + desc_elements = list(desc_iter) + + role_iter = map(lambda row: row.name, desc_elements) + assert set(role_iter) == {all_funcs_role, all_funcs_in_ks_role, specific_func_role} + + desc_iter = extract_create_statements(desc_elements) + assert set(desc_iter) == stmts + +def test_desc_service_levels_format(cql): + """ + Verify that the format of the output of `DESC SCHEMA WITH INTERNALS` corresponding to + creating service levels is of the expected form. + """ + + with AuthSLContext(cql): + sl_name = "my_service_level" + stmt = f"CREATE SERVICE LEVEL {sl_name};" + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_service_levels(desc_iter) + + [result] = list(desc_iter) + + assert result.keyspace_name == None + assert result.type == "service_level" + assert result.name == sl_name + assert result.create_statement == stmt + +def test_desc_service_levels_quotation_marks(cql): + """ + Verify that statements corresponding to creating service levels correctly format quotation marks. + """ + + with AuthSLContext(cql): + sl1_raw = "service \" 'level maybe'" + sl2_raw = "service ' \"level perhaps\"" + + sl1_single_quote = make_identifier(sl1_raw, quotation_mark="'") + sl1_double_quote = make_identifier(sl1_raw, quotation_mark='"') + sl2_double_quote = make_identifier(sl2_raw, quotation_mark='"') + + cql.execute(f"CREATE SERVICE LEVEL {sl1_single_quote}") + cql.execute(f"CREATE SERVICE LEVEL {sl2_double_quote}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_service_levels(desc_iter) + + desc_elements = list(desc_iter) + element_names_iter = extract_names(desc_elements) + + assert set(element_names_iter) == {sl1_double_quote, sl2_double_quote} + + desc_iter = extract_create_statements(desc_elements) + + expected_result = { + f"CREATE SERVICE LEVEL {sl1_double_quote};", + f"CREATE SERVICE LEVEL {sl2_double_quote};" + } + + assert set(desc_iter) == expected_result + +def test_desc_service_levels_uppercase(cql): + """ + Verify that statements corresponding to creating service levels correctly format uppercase characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + sl = '"myServiceLevel"' + cql.execute(f"CREATE SERVICE LEVEL {sl}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_service_levels(desc_iter) + + desc_elements = list(desc_iter) + + sl_iter = map(lambda row: row.name, desc_elements) + assert list(sl_iter) == [sl] + + desc_iter = extract_create_statements(desc_elements) + assert list(desc_iter) == [f"CREATE SERVICE LEVEL {sl};"] + +def test_desc_service_levels_unicode(cql): + """ + Verify that statements corresponding to creating service levels correctly format + unicode characters, i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + sl = '"レベル"' + cql.execute(f"CREATE SERVICE LEVEL {sl}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_service_levels(desc_iter) + + desc_elements = list(desc_iter) + + sl_iter = map(lambda row: row.name, desc_elements) + assert list(sl_iter) == [sl] + + desc_iter = extract_create_statements(desc_elements) + assert list(desc_iter) == [f"CREATE SERVICE LEVEL {sl};"] + +def test_desc_auth_service_levels(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to creating service levels + is as expected for various different cases. + """ + + with AuthSLContext(cql): + create_sl = ["CREATE SERVICE LEVEL {};"] + # Note: `CREATE SERVICE LEVEL` statements returned by `DESC SCHEMA WITH INTERNALS` always uses + # `std::chrono::milliseconds` as its resolution. For that reason, we use milliseconds in + # these statements too to reuse them later in the assert. + create_sl_time = [f"CREATE SERVICE LEVEL {{}} WITH TIMEOUT = {timeout};" for timeout in ["10ms", "350ms", "20000ms"]] + create_sl_wl_type = [f"CREATE SERVICE LEVEL {{}} WITH WORKLOAD_TYPE = '{work_type}';" + for work_type in ["interactive", "batch"]] + create_sl_time_and_wl_type = [f"CREATE SERVICE LEVEL {{}} WITH TIMEOUT = {timeout} AND WORKLOAD_TYPE = '{work_type}';" + for work_type in ["interactive", "batch"] for timeout in ["10ms", "350ms", "20000ms"]] + + create_sl_stmts = [*create_sl, *create_sl_time, *create_sl_wl_type, *create_sl_time_and_wl_type] + create_sl_stmts = [stmt.format(f'"my f@v0rit3 s3rv!c3 l3v3l!! !nd3x {idx}"') for idx, stmt in enumerate(create_sl_stmts)] + + for stmt in create_sl_stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_service_levels(desc_iter) + desc_iter = extract_create_statements(desc_iter) + + assert set(create_sl_stmts) == set(desc_iter) + +def test_desc_attach_service_level_format(cql): + """ + Verify that the format of the output of `DESC SCHEMA WITH INTERNALS` corresponding to + attaching service levels is of the expected form. + """ + + with AuthSLContext(cql): + role_name = "jasmine" + cql.execute(f"CREATE ROLE {role_name}") + + sl_name = "some_service_level" + cql.execute(f"CREATE SERVICE LEVEL {sl_name}") + + stmt = f"ATTACH SERVICE LEVEL {sl_name} TO {role_name};" + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_attached_service_levels(desc_iter) + + [result] = list(desc_iter) + + assert result.keyspace_name == None + assert result.type == "service_level_attachment" + assert result.name == sl_name + assert result.create_statement == stmt + +def test_desc_auth_attach_service_levels_quotation_marks(cql): + """ + Verify that statements corresponding to attaching service levels correctly format quotation marks. + """ + + with AuthSLContext(cql): + andrew_raw = "andrew \" 'the great'" + jane_raw = "jane ' \"the wise\"" + + andrew_single_quote = make_identifier(andrew_raw, quotation_mark="'") + andrew_double_quote = make_identifier(andrew_raw, quotation_mark='"') + jane_double_quote = make_identifier(jane_raw, quotation_mark='"') + + cql.execute(f"CREATE ROLE {andrew_single_quote}") + cql.execute(f"CREATE ROLE {jane_double_quote}") + + sl1_raw = "service \" 'level maybe'" + sl2_raw = "service ' \"level perhaps\"" + + sl1_single_quote = make_identifier(sl1_raw, quotation_mark="'") + sl1_double_quote = make_identifier(sl1_raw, quotation_mark='"') + sl2_double_quote = make_identifier(sl2_raw, quotation_mark='"') + + cql.execute(f"CREATE SERVICE LEVEL {sl1_single_quote}") + cql.execute(f"CREATE SERVICE LEVEL {sl2_double_quote}") + + cql.execute(f"ATTACH SERVICE LEVEL {sl1_single_quote} TO {andrew_single_quote}") + cql.execute(f"ATTACH SERVICE LEVEL {sl2_double_quote} TO {jane_double_quote}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_attached_service_levels(desc_iter) + + desc_elements = list(desc_iter) + element_names_iter = extract_names(desc_elements) + + assert set(element_names_iter) == {sl1_double_quote, sl2_double_quote} + + desc_iter = extract_create_statements(desc_elements) + + expected_result = { + f"ATTACH SERVICE LEVEL {sl1_double_quote} TO {andrew_double_quote};", + f"ATTACH SERVICE LEVEL {sl2_double_quote} TO {jane_double_quote};" + } + + assert set(desc_iter) == expected_result + +def test_desc_auth_attach_service_levels_uppercase(cql): + """ + Verify that statements corresponding to attaching service levels correctly format uppercase characters, + i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + role = '"myRole"' + sl = '"MyServiceLevel"' + + cql.execute(f"CREATE ROLE {role}") + cql.execute(f"CREATE SERVICE LEVEL {sl}") + cql.execute(f"ATTACH SERVICE LEVEL {sl} TO {role}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_attached_service_levels(desc_iter) + + desc_elements = list(desc_iter) + + sl_iter = map(lambda row: row.name, desc_elements) + assert list(sl_iter) == [sl] + + desc_iter = extract_create_statements(desc_elements) + assert list(desc_iter) == [f"ATTACH SERVICE LEVEL {sl} TO {role};"] + +def test_desc_attach_service_levels_unicode(cql): + """ + Verify that statements corresponding to attaching service levels correctly format + unicode characters, i.e. identifiers like that should be wrapped in quotation marks. + """ + + with AuthSLContext(cql): + role = '"私の猫"' + sl = '"サービスレベル"' + + cql.execute(f"CREATE ROLE {role}") + cql.execute(f"CREATE SERVICE LEVEL {sl}") + cql.execute(f"ATTACH SERVICE LEVEL {sl} TO {role}") + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_attached_service_levels(desc_iter) + + desc_elements = list(desc_iter) + + sl_iter = map(lambda row: row.name, desc_elements) + assert list(sl_iter) == [sl] + + desc_iter = extract_create_statements(desc_elements) + assert list(desc_iter) == [f"ATTACH SERVICE LEVEL {sl} TO {role};"] + +def test_desc_auth_attach_service_levels(cql): + """ + Verify that the output of `DESC SCHEMA WITH INTERNALS` corresponding to attaching service levels + is as expected for various different cases. + """ + + with AuthSLContext(cql): + [r1, r2] = ["andrew", '"j@n3 is my fr!3nd"'] + cql.execute(f"CREATE ROLE {r1}") + cql.execute(f"CREATE ROLE {r2}") + cql.execute(f"GRANT {r1} TO {r2}") + + [sl1, sl2] = ["my_service_level", '"s0m3 0th3r s3rv!c3 l3v3l!"'] + # Note: The smaller timeout, the better. We also want to verify in this test + # that the service level statement returned by `DESC SCHEMA` that corresponds + # to `r1` is the actual service level it was granted, not its effective service level. + cql.execute(f"CREATE SERVICE LEVEL {sl1} WITH TIMEOUT = 1ms") + cql.execute(f"CREATE SERVICE LEVEL {sl2} WITH TIMEOUT = 10ms") + + sl_stmt1 = f"ATTACH SERVICE LEVEL {sl1} TO {r1};" + sl_stmt2 = f"ATTACH SERVICE LEVEL {sl2} TO {r2};" + sl_stmts = [sl_stmt1, sl_stmt2] + + for stmt in sl_stmts: + cql.execute(stmt) + + desc_iter = cql.execute("DESC SCHEMA WITH INTERNALS") + desc_iter = filter_attached_service_levels(desc_iter) + desc_iter = extract_create_statements(desc_iter) + + assert set(sl_stmts) == set(desc_iter) + +def test_desc_restore(cql): + """ + Verify that restoring the schema, auth and service levels works correctly. We create entities + of each relevant kind, describe the schema, and drop everything. Then we restore it by + executing the obtained create statement, describe the schema again, and verify that we + have obtained the same description as before. + """ + + restore_stmts = None + ks = "my_ks" + + with AuthSLContext(cql, ks=ks): + cql.execute(f"CREATE TABLE {ks}.my_table (pk int PRIMARY KEY, sth int)") + cql.execute(f"CREATE TYPE {ks}.my_type (value int)") + cql.execute(f"""CREATE TABLE {ks}.some_other_table (c1 frozen, c2 double, c3 int, c4 set, + PRIMARY KEY ((c1, c2), c3)) WITH comment = 'some comment'""") + + cql.execute(f"""CREATE MATERIALIZED VIEW {ks}.mv AS + SELECT pk FROM {ks}.my_table + WHERE pk IS NOT NULL + PRIMARY KEY (pk) + WITH comment='some other comment'""") + + cql.execute(f"CREATE INDEX myindex ON {ks}.some_other_table (c1)") + + cql.execute(f"""CREATE FUNCTION {ks}.my_udf(val1 int, val2 int) + RETURNS NULL ON NULL INPUT + RETURNS int + LANGUAGE lua + AS $$ return val1 + val2 $$""") + cql.execute(f"""CREATE AGGREGATE {ks}.my_aggregate(int) + SFUNC my_udf + STYPE int""") + + [r1, r2, r3] = ["jack", "'b0b @nd d0b!'", "jane"] + cql.execute(f"CREATE ROLE {r1} WITH PASSWORD = 'pass1'") + cql.execute(f"CREATE ROLE {r2} WITH PASSWORD = 'pass2'") + cql.execute(f"CREATE ROLE {r3}") + + cql.execute(f"GRANT {r1} TO {r3}") + cql.execute(f"GRANT {r3} TO {r2}") + + cql.execute(f"GRANT ALL ON ALL KEYSPACES TO {r2}") + cql.execute(f"GRANT SELECT ON KEYSPACE {ks} TO {r3}") + cql.execute(f"GRANT MODIFY ON TABLE system.roles TO {r1}") + cql.execute(f"GRANT AUTHORIZE ON ALL ROLES TO {r1}") + cql.execute(f"GRANT DESCRIBE ON ALL ROLES TO {r1}") + + [sl1, sl2] = ["my_service_level", "'s3rv!c3 l3v3l !!!'"] + cql.execute(f"CREATE SERVICE LEVEL {sl1} WITH TIMEOUT = 10ms AND WORKLOAD_TYPE = 'batch'") + cql.execute(f"CREATE SERVICE LEVEL {sl2} WITH TIMEOUT = 100s") + + cql.execute(f"ATTACH SERVICE LEVEL {sl1} TO {r1}") + cql.execute(f"ATTACH SERVICE LEVEL {sl1} TO {r2}") + cql.execute(f"ATTACH SERVICE LEVEL {sl2} TO {r3}") + + restore_stmts = list(cql.execute("DESC SCHEMA WITH INTERNALS AND PASSWORDS")) + + class DescRow: + def __init__(self, row: DescRowType): + self.keyspace_name = row.keyspace_name + self.type = row.type + self.name = row.name + self.create_statement = row.create_statement + + def __eq__(self, other): + if isinstance(other, self.__class__): + return self.__dict__ == other.__dict__ + else: + return False + + def process_rows(rows: Iterable[DescRowType]) -> Iterable[DescRow]: + def remove_other_keyspaces(rows: Iterable[DescRow]) -> Iterable[DescRow]: + return filter(lambda row: row.keyspace_name == ks or row.keyspace_name == None, rows) + + def remove_ids_from_mv_rows(rows: Iterable[DescRow]) -> Iterable[DescRow]: + def aux(row: DescRow) -> DescRow: + if row.type == "view": + # ID is always the first option. + row.create_statement = re.sub(r"WITH ID = [^\n]+\n\s*AND", "WITH", row.create_statement) + return row + + return map(aux, rows) + + # We need to map the rows to `DescRow` to be able to modify its fields (cf. `remove_ids_from_mv_rows()`). + row_iter = map(lambda row: DescRow(row), rows) + # The statements used for restoring the keyspaces are not sorted. + # Other keyspaces might've been created by other tests, so we exclude them here. + row_iter = remove_other_keyspaces(row_iter) + # See: scylladb/scylladb#20616. + row_iter = remove_ids_from_mv_rows(row_iter) + + return row_iter + + restore_stmts = list(process_rows(restore_stmts)) + + with AuthSLContext(cql): + try: + for stmt in extract_create_statements(restore_stmts): + cql.execute(stmt) + + res = list(cql.execute("DESC SCHEMA WITH INTERNALS AND PASSWORDS")) + # The statements responsible for creating keyspaces are not sorted. + # Since other tests might've created more keyspaces, we exclude them here. + res = list(process_rows(res)) + + assert restore_stmts == res + finally: + cql.execute(f"DROP KEYSPACE IF EXISTS {ks}") diff --git a/test/lib/random_schema.cc b/test/lib/random_schema.cc index b6a81ba3cb..a9525ab9ad 100644 --- a/test/lib/random_schema.cc +++ b/test/lib/random_schema.cc @@ -11,6 +11,7 @@ #include #include "cql3/cql3_type.hh" +#include "cql3/description.hh" #include "mutation/mutation.hh" #include "schema/schema_builder.hh" #include "test/lib/cql_test_env.hh" @@ -844,9 +845,8 @@ schema_ptr build_random_schema(uint32_t seed, random_schema_specification& spec) } sstring udt_to_str(const user_type_impl& udt) { - std::stringstream ss; - udt.describe(ss); - return ss.str(); + auto udt_desc = udt.describe(cql3::with_create_statement::yes); + return *udt_desc.create_statement; } struct udt_list { @@ -1146,10 +1146,9 @@ future<> random_schema::create_with_cql(cql_test_env& env) { auto& db = env.local_db(); - std::stringstream ss; - _schema->describe(db, ss, false); + auto schema_desc = _schema->describe(db, cql3::describe_option::STMTS); - env.execute_cql(ss.str()).get(); + env.execute_cql(*schema_desc.create_statement).get(); auto& tbl = db.find_column_family(ks_name, tbl_name); _schema = tbl.schema(); diff --git a/test/lib/simple_schema.hh b/test/lib/simple_schema.hh index 0cb6eba150..6dcd04035b 100644 --- a/test/lib/simple_schema.hh +++ b/test/lib/simple_schema.hh @@ -97,7 +97,7 @@ public: sstring cql() const { return format("CREATE TABLE {}.{} (pk text, ck text, v text, s1 text{}{}, PRIMARY KEY (pk, ck))", - _s->keypace_name(), _s->element_name(), + _s->ks_name(), _s->cf_name(), _ws ? " static" : "", _wc ? ", c1 map" : ""); } diff --git a/tools/schema_loader.cc b/tools/schema_loader.cc index e758a75e6e..7291267e76 100644 --- a/tools/schema_loader.cc +++ b/tools/schema_loader.cc @@ -408,7 +408,7 @@ schema_ptr do_load_schema_from_schema_tables(const db::config& dbcfg, std::files return read_mutation_from_table_offline( sst_man, rcs_sem.make_tracking_only_permit(s, "schema_mutation", db::no_timeout, {}), - get_table_directory(scylla_data_path, s->keypace_name(), s->cf_name()).get(), + get_table_directory(scylla_data_path, s->ks_name(), s->cf_name()).get(), keyspace, schema_factory, data_value(keyspace), @@ -433,7 +433,7 @@ schema_ptr do_load_schema_from_schema_tables(const db::config& dbcfg, std::files auto types_mut = read_mutation_from_table_offline( sst_man, rcs_sem.make_tracking_only_permit(db::schema_tables::types(), "types_mutation", db::no_timeout, {}), - get_table_directory(scylla_data_path, types_schema->keypace_name(), types_schema->cf_name()).get(), + get_table_directory(scylla_data_path, types_schema->ks_name(), types_schema->cf_name()).get(), keyspace, db::schema_tables::types, data_value(keyspace), diff --git a/types/types.cc b/types/types.cc index 766b6b7873..f9dc6e2f0e 100644 --- a/types/types.cc +++ b/types/types.cc @@ -9,6 +9,7 @@ #include #include #include "cql3/cql3_type.hh" +#include "cql3/description.hh" #include "cql3/lists.hh" #include "cql3/maps.hh" #include "cql3/sets.hh" @@ -1064,6 +1065,10 @@ const sstring& abstract_type::cql3_type_name() const { return _cql3_type_name; } +sstring abstract_type::cql3_type_name_without_frozen() const { + return cql3_type_name_impl(*this); +} + void write_collection_value(bytes::iterator& out, data_type type, const data_value& value) { if (value.is_null()) { auto val_len = -1; @@ -3230,18 +3235,33 @@ sstring user_type_impl::get_name_as_cql_string() const { return cql3::util::maybe_quote(get_name_as_string()); } -std::ostream& user_type_impl::describe(std::ostream& os) const { - os << "CREATE TYPE " << cql3::util::maybe_quote(_keyspace) << "." << get_name_as_cql_string() << " (\n"; - for (size_t i = 0; i < _string_field_names.size(); i++) { - os << " " << cql3::util::maybe_quote(_string_field_names[i]) << " " << _types[i]->cql3_type_name(); - if (i < _string_field_names.size() - 1) { - os << ","; +cql3::description user_type_impl::describe(cql3::with_create_statement with_create_statement) const { + auto maybe_create_statement = std::invoke([&] -> std::optional { + if (!with_create_statement) { + return std::nullopt; } - os << "\n"; - } - os << ");"; - return os; + std::ostringstream os; + + os << "CREATE TYPE " << cql3::util::maybe_quote(_keyspace) << "." << get_name_as_cql_string() << " (\n"; + for (size_t i = 0; i < _string_field_names.size(); i++) { + os << " " << cql3::util::maybe_quote(_string_field_names[i]) << " " << _types[i]->cql3_type_name(); + if (i < _string_field_names.size() - 1) { + os << ","; + } + os << "\n"; + } + os << ");"; + + return std::move(os).str(); + }); + + return cql3::description { + .keyspace = _keyspace, + .type = "type", + .name = get_name_as_string(), + .create_statement = std::move(maybe_create_statement) + }; } data_type diff --git a/types/types.hh b/types/types.hh index 93e2b72b69..907dfd042e 100644 --- a/types/types.hh +++ b/types/types.hh @@ -453,6 +453,9 @@ public: bool is_native() const; cql3::cql3_type as_cql3_type() const; const sstring& cql3_type_name() const; + // The type is guaranteed to be wrapped within double quotation marks + // if it couldn't be used as a type identifier in CQL otherwise. + sstring cql3_type_name_without_frozen() const; virtual shared_ptr freeze() const { return shared_from_this(); } const abstract_type& without_reversed() const { diff --git a/types/user.hh b/types/user.hh index 52468fb7eb..c2d853ce89 100644 --- a/types/user.hh +++ b/types/user.hh @@ -8,11 +8,11 @@ #pragma once +#include "cql3/description.hh" #include "types/types.hh" #include "types/tuple.hh" -#include "data_dictionary/keyspace_element.hh" -class user_type_impl : public tuple_type_impl, public data_dictionary::keyspace_element { +class user_type_impl : public tuple_type_impl { using intern = type_interning_helper, std::vector, bool>; public: const sstring _keyspace; @@ -59,10 +59,7 @@ public: */ std::set get_all_referenced_user_types() const; - virtual sstring keypace_name() const override { return _keyspace; } - virtual sstring element_name() const override { return get_name_as_string(); } - virtual sstring element_type() const override { return "type"; } - virtual std::ostream& describe(std::ostream& os) const override; + cql3::description describe(cql3::with_create_statement) const; private: static sstring make_name(sstring keyspace,