diff options
author | Jean-Sébastien Pédron <jean-sebastien@rabbitmq.com> | 2022-07-15 17:42:04 +0200 |
---|---|---|
committer | Jean-Sébastien Pédron <jean-sebastien@rabbitmq.com> | 2022-08-04 09:34:14 +0200 |
commit | 3b989d0f335471085a594ed286661c4443a69e16 (patch) | |
tree | 23aea246414b24c558113dc878ffd19a6094a7ae | |
parent | 357d79fba6e93d4c10b88f5cbdb4f7b3187000b6 (diff) | |
download | rabbitmq-server-git-3b989d0f335471085a594ed286661c4443a69e16.tar.gz |
Remove pre-user_limits compatibility code
User limits required a breaking change protected behind a feature flag.
This allowed a RabbitMQ cluster to be upgraded one node at a time,
without having to stop the entire cluster.
The breaking change was a new field in the `#internal_user{}` record. This
broke the API and the ABI because records are a compile-time thing in
Erlang.
The compatibility code is in the wild for long enough that we want to
support the new `#internal_user{}` record only from now on. The
`user_limits` feature flag was marked as required in a previous commit
(see #5202). This allows us to remove code in this patch.
References #5215.
(cherry picked from commit eeaf8d39e55d348ac6027c2d239b1a1401c15e97)
# Conflicts:
# deps/rabbit/src/rabbit_auth_backend_internal.erl
-rw-r--r-- | deps/rabbit/src/internal_user.erl | 133 | ||||
-rw-r--r-- | deps/rabbit/src/internal_user_v1.erl | 151 | ||||
-rw-r--r-- | deps/rabbit/src/rabbit_auth_backend_internal.erl | 44 |
3 files changed, 58 insertions, 270 deletions
diff --git a/deps/rabbit/src/internal_user.erl b/deps/rabbit/src/internal_user.erl index fdb65a485d..943058ac5a 100644 --- a/deps/rabbit/src/internal_user.erl +++ b/deps/rabbit/src/internal_user.erl @@ -36,7 +36,7 @@ -type(password_hash() :: binary()). --type internal_user() :: internal_user_v1:internal_user_v1() | internal_user_v2(). +-type internal_user() :: internal_user_v2(). -record(internal_user, { username :: username() | '_', @@ -55,8 +55,7 @@ hashing_algorithm :: atom() | '_', limits :: map()}). --type internal_user_pattern() :: internal_user_v1:internal_user_v1_pattern() | - internal_user_v2_pattern(). +-type internal_user_pattern() :: internal_user_v2_pattern(). -type internal_user_v2_pattern() :: #internal_user{ username :: username() | '_', @@ -75,129 +74,85 @@ -spec new() -> internal_user(). new() -> - case record_version_to_use() of - ?record_version -> - #internal_user{ - username = <<"">>, - password_hash = <<"">>, - tags = [] - }; - _ -> - internal_user_v1:new() - end. + #internal_user{ + username = <<"">>, + password_hash = <<"">>, + tags = [] + }. -spec new(tuple()) -> internal_user(). new({hashing_algorithm, HashingAlgorithm}) -> - case record_version_to_use() of - ?record_version -> - #internal_user{ - username = <<"">>, - password_hash = <<"">>, - tags = [], - hashing_algorithm = HashingAlgorithm - }; - _ -> - internal_user_v1:new({hashing_algorithm, HashingAlgorithm}) - end; + #internal_user{ + username = <<"">>, + password_hash = <<"">>, + tags = [], + hashing_algorithm = HashingAlgorithm + }; new({tags, Tags}) -> - case record_version_to_use() of - ?record_version -> - #internal_user{ - username = <<"">>, - password_hash = <<"">>, - tags = Tags - }; - _ -> - internal_user_v1:new({tags, Tags}) - end. - --spec record_version_to_use() -> internal_user_v1 | internal_user_v2. + #internal_user{ + username = <<"">>, + password_hash = <<"">>, + tags = Tags + }. + +-spec record_version_to_use() -> internal_user_v2. record_version_to_use() -> - case rabbit_feature_flags:is_enabled(user_limits) of - true -> ?record_version; - false -> internal_user_v1:record_version_to_use() - end. + ?record_version. -spec fields() -> list(). fields() -> - case record_version_to_use() of - ?record_version -> fields(?record_version); - _ -> internal_user_v1:fields() - end. + fields(?record_version). -spec fields(atom()) -> list(). -fields(?record_version) -> record_info(fields, internal_user); -fields(Version) -> internal_user_v1:fields(Version). +fields(?record_version) -> record_info(fields, internal_user). -spec upgrade(internal_user()) -> internal_user(). -upgrade(#internal_user{} = User) -> User; -upgrade(OldUser) -> upgrade_to(record_version_to_use(), OldUser). +upgrade(#internal_user{} = User) -> User. --spec upgrade_to -(internal_user_v2, internal_user()) -> internal_user_v2(); -(internal_user_v1, internal_user_v1:internal_user_v1()) -> internal_user_v1:internal_user_v1(). +-spec upgrade_to(internal_user_v2, internal_user()) -> internal_user_v2(). upgrade_to(?record_version, #internal_user{} = User) -> User; upgrade_to(?record_version, OldUser) -> Fields = erlang:tuple_to_list(OldUser) ++ [#{}], - #internal_user{} = erlang:list_to_tuple(Fields); -upgrade_to(Version, OldUser) -> - internal_user_v1:upgrade_to(Version, OldUser). + #internal_user{} = erlang:list_to_tuple(Fields). -spec pattern_match_all() -> internal_user_pattern(). pattern_match_all() -> - case record_version_to_use() of - ?record_version -> #internal_user{_ = '_'}; - _ -> internal_user_v1:pattern_match_all() - end. + #internal_user{_ = '_'}. -spec get_username(internal_user()) -> username(). -get_username(#internal_user{username = Value}) -> Value; -get_username(User) -> internal_user_v1:get_username(User). +get_username(#internal_user{username = Value}) -> Value. -spec get_password_hash(internal_user()) -> password_hash(). -get_password_hash(#internal_user{password_hash = Value}) -> Value; -get_password_hash(User) -> internal_user_v1:get_password_hash(User). +get_password_hash(#internal_user{password_hash = Value}) -> Value. -spec get_tags(internal_user()) -> [atom()]. -get_tags(#internal_user{tags = Value}) -> Value; -get_tags(User) -> internal_user_v1:get_tags(User). +get_tags(#internal_user{tags = Value}) -> Value. -spec get_hashing_algorithm(internal_user()) -> atom(). -get_hashing_algorithm(#internal_user{hashing_algorithm = Value}) -> Value; -get_hashing_algorithm(User) -> internal_user_v1:get_hashing_algorithm(User). +get_hashing_algorithm(#internal_user{hashing_algorithm = Value}) -> Value. -spec get_limits(internal_user()) -> map(). -get_limits(#internal_user{limits = Value}) -> Value; -get_limits(User) -> internal_user_v1:get_limits(User). +get_limits(#internal_user{limits = Value}) -> Value. -spec create_user(username(), password_hash(), atom()) -> internal_user(). create_user(Username, PasswordHash, HashingMod) -> - case record_version_to_use() of - ?record_version -> - #internal_user{username = Username, - password_hash = PasswordHash, - tags = [], - hashing_algorithm = HashingMod, - limits = #{} - }; - _ -> - internal_user_v1:create_user(Username, PasswordHash, HashingMod) - end. + #internal_user{username = Username, + password_hash = PasswordHash, + tags = [], + hashing_algorithm = HashingMod, + limits = #{} + }. -spec set_password_hash(internal_user(), password_hash(), atom()) -> internal_user(). set_password_hash(#internal_user{} = User, PasswordHash, HashingAlgorithm) -> User#internal_user{password_hash = PasswordHash, - hashing_algorithm = HashingAlgorithm}; -set_password_hash(User, PasswordHash, HashingAlgorithm) -> - internal_user_v1:set_password_hash(User, PasswordHash, HashingAlgorithm). + hashing_algorithm = HashingAlgorithm}. -spec set_tags(internal_user(), [atom()]) -> internal_user(). set_tags(#internal_user{} = User, Tags) -> - User#internal_user{tags = Tags}; -set_tags(User, Tags) -> - internal_user_v1:set_tags(User, Tags). + User#internal_user{tags = Tags}. -spec update_limits (add, internal_user(), map()) -> internal_user(); @@ -205,12 +160,8 @@ set_tags(User, Tags) -> update_limits(add, #internal_user{limits = Limits} = User, Term) -> User#internal_user{limits = maps:merge(Limits, Term)}; update_limits(remove, #internal_user{limits = Limits} = User, LimitType) -> - User#internal_user{limits = maps:remove(LimitType, Limits)}; -update_limits(Action, User, Term) -> - internal_user_v1:update_limits(Action, User, Term). + User#internal_user{limits = maps:remove(LimitType, Limits)}. -spec clear_limits(internal_user()) -> internal_user(). clear_limits(#internal_user{} = User) -> - User#internal_user{limits = #{}}; -clear_limits(User) -> - internal_user_v1:clear_limits(User). + User#internal_user{limits = #{}}. diff --git a/deps/rabbit/src/internal_user_v1.erl b/deps/rabbit/src/internal_user_v1.erl deleted file mode 100644 index 4a2be0462a..0000000000 --- a/deps/rabbit/src/internal_user_v1.erl +++ /dev/null @@ -1,151 +0,0 @@ -%% This Source Code Form is subject to the terms of the Mozilla Public -%% License, v. 2.0. If a copy of the MPL was not distributed with this -%% file, You can obtain one at https://mozilla.org/MPL/2.0/. -%% -%% Copyright (c) 2020-2022 VMware, Inc. or its affiliates. All rights reserved. -%% - --module(internal_user_v1). - --include_lib("rabbit_common/include/rabbit.hrl"). - --export([ - new/0, - new/1, - record_version_to_use/0, - fields/0, - fields/1, - upgrade/1, - upgrade_to/2, - pattern_match_all/0, - get_username/1, - get_password_hash/1, - get_tags/1, - get_hashing_algorithm/1, - get_limits/1, - create_user/3, - set_password_hash/3, - set_tags/2, - update_limits/3, - clear_limits/1 -]). - --define(record_version, ?MODULE). - --record(internal_user, { - username :: internal_user:username() | '_', - password_hash :: internal_user:password_hash() | '_', - tags :: [atom()] | '_', - %% password hashing implementation module, - %% typically rabbit_password_hashing_* but can - %% come from a plugin - hashing_algorithm :: atom() | '_'}). - --type internal_user() :: internal_user_v1(). - --type(internal_user_v1() :: - #internal_user{username :: internal_user:username(), - password_hash :: internal_user:password_hash(), - tags :: [atom()], - hashing_algorithm :: atom()}). - --type internal_user_pattern() :: internal_user_v1_pattern(). - --type internal_user_v1_pattern() :: #internal_user{ - username :: internal_user:username() | '_', - password_hash :: '_', - tags :: '_', - hashing_algorithm :: '_' - }. - --export_type([internal_user/0, - internal_user_v1/0, - internal_user_pattern/0, - internal_user_v1_pattern/0]). - --spec record_version_to_use() -> internal_user_v1. -record_version_to_use() -> - ?record_version. - --spec new() -> internal_user(). -new() -> - #internal_user{ - username = <<"">>, - password_hash = <<"">>, - tags = [] - }. - --spec new(tuple()) -> internal_user(). -new({hashing_algorithm, HashingAlgorithm}) -> - #internal_user{ - username = <<"">>, - password_hash = <<"">>, - hashing_algorithm = HashingAlgorithm, - tags = [] - }; -new({tags, Tags}) -> - #internal_user{ - username = <<"">>, - password_hash = <<"">>, - tags = Tags - }. - --spec fields() -> list(). -fields() -> fields(?record_version). - --spec fields(atom()) -> list(). -fields(?record_version) -> record_info(fields, internal_user). - --spec upgrade(internal_user()) -> internal_user(). -upgrade(#internal_user{} = User) -> User. - --spec upgrade_to(internal_user_v1, internal_user()) -> internal_user(). -upgrade_to(?record_version, #internal_user{} = User) -> - User. - --spec pattern_match_all() -> internal_user_pattern(). -pattern_match_all() -> #internal_user{_ = '_'}. - --spec get_username(internal_user()) -> internal_user:username(). -get_username(#internal_user{username = Value}) -> Value. - --spec get_password_hash(internal_user()) -> internal_user:password_hash(). -get_password_hash(#internal_user{password_hash = Value}) -> Value. - --spec get_tags(internal_user()) -> [atom()]. -get_tags(#internal_user{tags = Value}) -> Value. - --spec get_hashing_algorithm(internal_user()) -> atom(). -get_hashing_algorithm(#internal_user{hashing_algorithm = Value}) -> Value. - --spec get_limits(internal_user()) -> map(). -get_limits(_User) -> #{}. - --spec create_user(internal_user:username(), internal_user:password_hash(), - atom()) -> internal_user(). -create_user(Username, PasswordHash, HashingMod) -> - #internal_user{username = Username, - password_hash = PasswordHash, - tags = [], - hashing_algorithm = HashingMod - }. - --spec set_password_hash(internal_user:internal_user(), - internal_user:password_hash(), atom()) -> internal_user(). -set_password_hash(#internal_user{} = User, PasswordHash, HashingAlgorithm) -> - User#internal_user{password_hash = PasswordHash, - hashing_algorithm = HashingAlgorithm}. - --spec set_tags(internal_user(), [atom()]) -> internal_user(). -set_tags(#internal_user{} = User, Tags) -> - User#internal_user{tags = Tags}. - --spec update_limits -(add, internal_user(), map()) -> internal_user(); -(remove, internal_user(), term()) -> internal_user(). -update_limits(_, User, _) -> - User. - --spec clear_limits(internal_user()) -> internal_user(). -clear_limits(User) -> - User. diff --git a/deps/rabbit/src/rabbit_auth_backend_internal.erl b/deps/rabbit/src/rabbit_auth_backend_internal.erl index 645d2dbbd4..cf6d4963c0 100644 --- a/deps/rabbit/src/rabbit_auth_backend_internal.erl +++ b/deps/rabbit/src/rabbit_auth_backend_internal.erl @@ -752,20 +752,15 @@ put_user(User, Version, ActingUser) -> rabbit_credential_validation:validate(Username, Password) =:= ok end, - Limits = case rabbit_feature_flags:is_enabled(user_limits) of - false -> - undefined; - true -> - case maps:get(limits, User, undefined) of - undefined -> - undefined; - Term -> - case validate_user_limits(Term) of - ok -> Term; - Error -> throw(Error) - end - end - end, + Limits = case maps:get(limits, User, undefined) of + undefined -> + undefined; + Term -> + case validate_user_limits(Term) of + ok -> Term; + Error -> throw(Error) + end + end, case exists(Username) of true -> case {HasPassword, HasPasswordHash} of @@ -847,22 +842,15 @@ preconfigure_permissions(Username, Map, ActingUser) when is_map(Map) -> ok. set_user_limits(Username, Definition, ActingUser) when is_list(Definition); is_binary(Definition) -> - case rabbit_feature_flags:is_enabled(user_limits) of - true -> - case rabbit_json:try_decode(rabbit_data_coercion:to_binary(Definition)) of - {ok, Term} -> - validate_parameters_and_update_limit(Username, Term, ActingUser); - {error, Reason} -> - {error_string, - rabbit_misc:format("Could not parse JSON document: ~tp", [Reason])} - end; - false -> {error_string, "cannot set any user limits: the user_limits feature flag is not enabled"} + case rabbit_json:try_decode(rabbit_data_coercion:to_binary(Definition)) of + {ok, Term} -> + validate_parameters_and_update_limit(Username, Term, ActingUser); + {error, Reason} -> + {error_string, + rabbit_misc:format("Could not parse JSON document: ~tp", [Reason])} end; set_user_limits(Username, Definition, ActingUser) when is_map(Definition) -> - case rabbit_feature_flags:is_enabled(user_limits) of - true -> validate_parameters_and_update_limit(Username, Definition, ActingUser); - false -> {error_string, "cannot set any user limits: the user_limits feature flag is not enabled"} - end. + validate_parameters_and_update_limit(Username, Definition, ActingUser). validate_parameters_and_update_limit(Username, Term, ActingUser) -> case validate_user_limits(Term) of |