Add TODO for queue limit validation

Validation code in place, how to display?

Take existing queue count into consideration

Server-side validation takes existing queue count into consideration. Improve error message sent back to web application.

Improve messaging, remove unused template

Take existing queues into account when importing definitions for all vhosts. Fix test suite.
This commit is contained in:
Luke Bakken 2017-06-25 15:29:47 -07:00
parent 62af4ac754
commit 47b70d8a8a
5 changed files with 120 additions and 33 deletions

View File

@ -259,9 +259,6 @@ dispatcher_add(function(sammy) {
location.reload();
});
sammy.get('#/import-succeeded', function() {
render({}, 'import-succeeded', '#/overview');
});
sammy.put('#/rate-options', function() {
update_rate_options(this);
});

View File

@ -519,26 +519,34 @@ function show_popup(type, text, mode) {
});
}
function submit_import(form) {
if (form.file.value) {
var confirm_upload = confirm('Are you sure you want to import a definitions file? Some entities (vhosts, users, queues, etc) may be overwritten!');
if (confirm_upload === true) {
var idx = $("select[name='vhost-upload'] option:selected").index();
var vhost = ((idx <= 0) ? "" : "/" + esc($("select[name='vhost-upload'] option:selected").val()));
form.action ="api/definitions" + vhost + '?auth=' + get_cookie_value('auth');
form.submit();
window.location.replace("../../#/import-succeeded");
} else {
return false;
}
} else {
return false;
}
};
var file = form.file.files[0]; // FUTURE: limit upload file size (?)
var vhost_upload = $("select[name='vhost-upload'] option:selected");
var vhost_selected = vhost_upload.index() > 0;
var vhost_name = null;
if (vhost_selected) {
vhost_name = vhost_upload.val();
}
var vhost_part = '';
if (vhost_name) {
vhost_part = '/' + esc(vhost_name);
}
var form_action = "/definitions" + vhost_part + '?auth=' + get_cookie_value('auth');
var fd = new FormData();
fd.append('file', file);
with_req('POST', form_action, fd, function(resp) {
show_popup('info', 'Your definitions were imported successfully.');
});
}
}
return false;
};
function postprocess() {
$('form.confirm-queue').submit(function() {
@ -1077,7 +1085,7 @@ function has_auth_cookie_value() {
function auth_header() {
if(has_auth_cookie_value()) {
return "Basic " + decodeURIComponent(get_cookie_value('auth'));
return "Basic " + decodeURIComponent(get_cookie_value('auth'));
} else {
return null;
}

View File

@ -1,4 +0,0 @@
<h1>Import succeeded</h1>
<p>
Your definitions were imported successfully.
</p>

View File

@ -178,6 +178,7 @@ apply_defs(Body, Username, SuccessFun, ErrorFun) ->
Username)
end),
for_all(vhosts, Username, All, fun add_vhost/2),
validate_limits(All),
for_all(permissions, Username, All, fun add_permission/2),
for_all(topic_permissions, Username, All, fun add_topic_permission/2),
for_all(parameters, Username, All, fun add_parameter/2),
@ -198,6 +199,7 @@ apply_defs(Body, Username, SuccessFun, ErrorFun, VHost) ->
ErrorFun(E);
{ok, _, All} ->
try
validate_limits(All, VHost),
for_all(policies, Username, All, VHost, fun add_policy/3),
for_all(queues, Username, All, VHost, fun add_queue/3),
for_all(exchanges, Username, All, VHost, fun add_exchange/3),
@ -210,6 +212,15 @@ apply_defs(Body, Username, SuccessFun, ErrorFun, VHost) ->
format(#amqp_error{name = Name, explanation = Explanation}) ->
rabbit_data_coercion:to_binary(rabbit_misc:format("~s: ~s", [Name, Explanation]));
format({no_such_vhost, undefined}) ->
rabbit_data_coercion:to_binary(
"Virtual host is not specified in definitions file nor via management interface.");
format({no_such_vhost, VHost}) ->
rabbit_data_coercion:to_binary(
rabbit_misc:format("Please create virtual host \"~s\" prior to importing definitions.",
[VHost]));
format({vhost_limit_exceeded, ErrMsg}) ->
rabbit_data_coercion:to_binary(ErrMsg);
format(E) ->
rabbit_data_coercion:to_binary(rabbit_misc:format("~p", [E])).
@ -426,3 +437,69 @@ rv(VHost, Type, Props) -> rv(VHost, Type, name, Props).
rv(VHost, Type, Name, Props) ->
rabbit_misc:r(VHost, Type, maps:get(Name, Props, undefined)).
%%--------------------------------------------------------------------
validate_limits(All) ->
case maps:get(queues, All, undefined) of
undefined -> ok;
Queues0 ->
{ok, VHostMap} = filter_out_existing_queues(Queues0),
maps:fold(fun validate_vhost_limit/3, ok, VHostMap)
end.
validate_limits(All, VHost) ->
case maps:get(queues, All, undefined) of
undefined -> ok;
Queues0 ->
Queues1 = filter_out_existing_queues(VHost, Queues0),
AddCount = length(Queues1),
validate_vhost_limit(VHost, AddCount, ok)
end.
filter_out_existing_queues(Queues) ->
build_filtered_map(Queues, maps:new()).
filter_out_existing_queues(VHost, Queues) ->
Pred = fun(Queue) ->
Rec = rv(VHost, queue, <<"name">>, Queue),
case rabbit_amqqueue:lookup(Rec) of
{ok, _} -> false;
{error, not_found} -> true
end
end,
lists:filter(Pred, Queues).
build_queue_data(Queue) ->
VHost = maps:get(<<"vhost">>, Queue, undefined),
Rec = rv(VHost, queue, <<"name">>, Queue),
{Rec, VHost}.
build_filtered_map([], AccMap) ->
{ok, AccMap};
build_filtered_map([Queue|Rest], AccMap0) ->
{Rec, VHost} = build_queue_data(Queue),
case rabbit_amqqueue:lookup(Rec) of
{error, not_found} ->
AccMap1 = maps:update_with(VHost, fun(V) -> V + 1 end, 1, AccMap0),
build_filtered_map(Rest, AccMap1);
{ok, _} ->
build_filtered_map(Rest, AccMap0)
end.
validate_vhost_limit(VHost, AddCount, ok) ->
WouldExceed = rabbit_vhost_limit:would_exceed_queue_limit(AddCount, VHost),
validate_vhost_queue_limit(VHost, AddCount, WouldExceed).
validate_vhost_queue_limit(_VHost, 0, _) ->
% Note: not adding any new queues so the upload
% must be update-only
ok;
validate_vhost_queue_limit(_VHost, _AddCount, false) ->
% Note: would not exceed queue limit
ok;
validate_vhost_queue_limit(VHost, AddCount, {true, Limit, QueueCount}) ->
ErrFmt = "Adding ~B queue(s) to virtual host \"~s\" would exceed the limit of ~B queue(s).~n~nThis virtual host currently has ~B queue(s) defined.~n~nImport aborted!",
ErrInfo = [AddCount, VHost, Limit, QueueCount],
ErrMsg = rabbit_misc:format(ErrFmt, ErrInfo),
exit({vhost_limit_exceeded, ErrMsg}).

View File

@ -1039,9 +1039,15 @@ defs_v(Config, Key, URI, CreateMethod, Args) ->
http_put(Config, "/vhosts/test", none, {group, '2xx'}),
PermArgs = [{configure, <<".*">>}, {write, <<".*">>}, {read, <<".*">>}],
http_put(Config, "/permissions/test/guest", PermArgs, {group, '2xx'}),
defs(Config, Key, Rep1(URI, "test"), CreateMethod, ReplaceVHostInArgs(Args, <<"test">>),
fun(URI2) -> http_delete(Config, URI2, {group, '2xx'}),
http_delete(Config, "/vhosts/test", {group, '2xx'}) end).
DeleteFun0 = fun(URI2) ->
http_delete(Config, URI2, {group, '2xx'})
end,
DeleteFun1 = fun(_) ->
http_delete(Config, "/vhosts/test", {group, '2xx'})
end,
defs(Config, Key, Rep1(URI, "test"),
CreateMethod, ReplaceVHostInArgs(Args, <<"test">>),
DeleteFun0, DeleteFun1).
create(Config, CreateMethod, URI, Args) ->
case CreateMethod of
@ -1055,6 +1061,9 @@ create(Config, CreateMethod, URI, Args) ->
end.
defs(Config, Key, URI, CreateMethod, Args, DeleteFun) ->
defs(Config, Key, URI, CreateMethod, Args, DeleteFun, DeleteFun).
defs(Config, Key, URI, CreateMethod, Args, DeleteFun0, DeleteFun1) ->
%% Create the item
URI2 = create(Config, CreateMethod, URI, Args),
%% Make sure it ends up in definitions
@ -1062,14 +1071,14 @@ defs(Config, Key, URI, CreateMethod, Args, DeleteFun) ->
true = lists:any(fun(I) -> test_item(Args, I) end, maps:get(Key, Definitions)),
%% Delete it
DeleteFun(URI2),
DeleteFun0(URI2),
%% Post the definitions back, it should get recreated in correct form
http_post(Config, "/definitions", Definitions, {group, '2xx'}),
assert_item(Args, http_get(Config, URI2, ?OK)),
%% And delete it again
DeleteFun(URI2),
DeleteFun1(URI2),
passed.
@ -2028,6 +2037,7 @@ publish_fail_test(Config) ->
|| BadProp <- [{priority, <<"really high">>},
{timestamp, <<"recently">>},
{expiration, 1234}]],
http_delete(Config, "/queues/%2f/publish_fail_test", {group, '2xx'}),
http_delete(Config, "/users/myuser", {group, '2xx'}),
passed.
@ -2323,11 +2333,10 @@ vhost_limits_list_test(Config) ->
[] = http_get(Config, "/vhost-limits/limit_test_vhost_2", ?OK),
Limits1 = [#{vhost => <<"limit_test_vhost_1">>,
value => #{'max-connections' => 100, 'max-queues' => 100}}],
Limits2 = [#{vhost => <<"limit_test_vhost_2">>,
value => #{'max-connections' => 200}}],
Limits1 = [#{vhost => <<"limit_test_vhost_1">>,
value => #{'max-connections' => 100, 'max-queues' => 100}}],
Limits2 = [#{vhost => <<"limit_test_vhost_2">>,
value => #{'max-connections' => 200}}],
Expected = Limits1 ++ Limits2,