Use negative values to disable per-vhost limits, 0 to refuse all connections

Per recommendation from @dumbbell.

1 files changed, 88 insertions, 16 deletions

diff --git a/test/per_vhost_connection_limit_SUITE.erl b/test/per_vhost_connection_limit_SUITE.erl
index 033bd0abdd..f327e82cb7 100644
--- a/test/per_vhost_connection_limit_SUITE.erl
+++ b/test/per_vhost_connection_limit_SUITE.erl
@@ -40,7 +40,9 @@ groups() ->
           single_node_multiple_vhost_connection_count,
           single_node_list_in_vhost,
           single_node_single_vhost_limit,
+          single_node_single_vhost_zero_limit,
           single_node_multiple_vhost_limit,
+          single_node_multiple_vhost_zero_limit,
           single_node_vhost_deletion_forces_connection_closure
         ]},
       {cluster_size_2, [], [
@@ -398,6 +400,10 @@ cluster_node_list_on_node(Config) ->
     rabbit_ct_broker_helpers:start_broker(Config, 1).
 
 single_node_single_vhost_limit(Config) ->
+    single_node_single_vhost_limit_with(Config, 5),
+    single_node_single_vhost_limit_with(Config, -1).
+
+single_node_single_vhost_limit_with(Config, WatermarkLimit) ->
     VHost = <<"/">>,
     set_vhost_connection_limit(Config, VHost, 3),
 
@@ -408,11 +414,11 @@ single_node_single_vhost_limit(Config) ->
     Conn3 = open_unmanaged_connection(Config, 0),
 
     %% we've crossed the limit
-    {error, not_allowed} = open_unmanaged_connection(Config, 0),
-    {error, not_allowed} = open_unmanaged_connection(Config, 0),
-    {error, not_allowed} = open_unmanaged_connection(Config, 0),
+    expect_that_client_connection_is_rejected(Config, 0),
+    expect_that_client_connection_is_rejected(Config, 0),
+    expect_that_client_connection_is_rejected(Config, 0),
 
-    set_vhost_connection_limit(Config, VHost, 5),
+    set_vhost_connection_limit(Config, VHost, WatermarkLimit),
     Conn4 = open_unmanaged_connection(Config, 0),
     Conn5 = open_unmanaged_connection(Config, 0),
 
@@ -421,7 +427,29 @@ single_node_single_vhost_limit(Config) ->
                   end, [Conn1, Conn2, Conn3, Conn4, Conn5]),
 
     ?assertEqual(0, count_connections_in(Config, VHost)),
-    set_vhost_connection_limit(Config, VHost, 0).
+    set_vhost_connection_limit(Config, VHost,  -1).
+
+single_node_single_vhost_zero_limit(Config) ->
+    VHost = <<"/">>,
+    set_vhost_connection_limit(Config, VHost, 0),
+
+    ?assertEqual(0, count_connections_in(Config, VHost)),
+
+    %% with limit = 0 no connections are allowed
+    expect_that_client_connection_is_rejected(Config),
+    expect_that_client_connection_is_rejected(Config),
+    expect_that_client_connection_is_rejected(Config),
+
+    set_vhost_connection_limit(Config, VHost, -1),
+    Conn1 = open_unmanaged_connection(Config, 0),
+    Conn2 = open_unmanaged_connection(Config, 0),
+
+    lists:foreach(fun (C) ->
+                          rabbit_ct_client_helpers:close_connection(C)
+                  end, [Conn1, Conn2]),
+
+    ?assertEqual(0, count_connections_in(Config, VHost)).
+
 
 single_node_multiple_vhost_limit(Config) ->
     VHost1 = <<"vhost1">>,
@@ -442,13 +470,13 @@ single_node_multiple_vhost_limit(Config) ->
     Conn4 = open_unmanaged_connection(Config, 0, VHost2),
 
     %% we've crossed the limit
-    {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost1),
-    {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost2),
+    expect_that_client_connection_is_rejected(Config, 0, VHost1),
+    expect_that_client_connection_is_rejected(Config, 0, VHost2),
 
     Conn5 = open_unmanaged_connection(Config, 0),
 
     set_vhost_connection_limit(Config, VHost1, 5),
-    set_vhost_connection_limit(Config, VHost2, 5),
+    set_vhost_connection_limit(Config, VHost2, -10),
 
     Conn6 = open_unmanaged_connection(Config, 0, VHost1),
     Conn7 = open_unmanaged_connection(Config, 0, VHost1),
@@ -464,12 +492,47 @@ single_node_multiple_vhost_limit(Config) ->
     ?assertEqual(0, count_connections_in(Config, VHost1)),
     ?assertEqual(0, count_connections_in(Config, VHost2)),
 
-    set_vhost_connection_limit(Config, VHost1, 100000000),
-    set_vhost_connection_limit(Config, VHost2, 100000000),
+    set_vhost_connection_limit(Config, VHost1, -1),
+    set_vhost_connection_limit(Config, VHost2, -1),
 
     rabbit_ct_broker_helpers:delete_vhost(Config, VHost1),
     rabbit_ct_broker_helpers:delete_vhost(Config, VHost2).
 
+
+single_node_multiple_vhost_zero_limit(Config) ->
+    VHost1 = <<"vhost1">>,
+    VHost2 = <<"vhost2">>,
+
+    set_up_vhost(Config, VHost1),
+    set_up_vhost(Config, VHost2),
+
+    set_vhost_connection_limit(Config, VHost1, 0),
+    set_vhost_connection_limit(Config, VHost2, 0),
+
+    ?assertEqual(0, count_connections_in(Config, VHost1)),
+    ?assertEqual(0, count_connections_in(Config, VHost2)),
+
+    %% with limit = 0 no connections are allowed
+    expect_that_client_connection_is_rejected(Config, 0, VHost1),
+    expect_that_client_connection_is_rejected(Config, 0, VHost2),
+    expect_that_client_connection_is_rejected(Config, 0, VHost1),
+
+    set_vhost_connection_limit(Config, VHost1, -1),
+    Conn1 = open_unmanaged_connection(Config, 0, VHost1),
+    Conn2 = open_unmanaged_connection(Config, 0, VHost1),
+
+    lists:foreach(fun (C) ->
+                          rabbit_ct_client_helpers:close_connection(C)
+                  end, [Conn1, Conn2]),
+
+    ?assertEqual(0, count_connections_in(Config, VHost1)),
+    ?assertEqual(0, count_connections_in(Config, VHost2)),
+
+    set_vhost_connection_limit(Config, VHost1, -1),
+    set_vhost_connection_limit(Config, VHost2, -1).
+
+
+
 cluster_single_vhost_limit(Config) ->
     VHost = <<"/">>,
     set_vhost_connection_limit(Config, VHost, 2),
@@ -483,8 +546,8 @@ cluster_single_vhost_limit(Config) ->
     timer:sleep(200),
 
     %% we've crossed the limit
-    {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost),
-    {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost),
+    expect_that_client_connection_is_rejected(Config, 0, VHost),
+    expect_that_client_connection_is_rejected(Config, 1, VHost),
 
     set_vhost_connection_limit(Config, VHost, 5),
 
@@ -497,7 +560,7 @@ cluster_single_vhost_limit(Config) ->
 
     ?assertEqual(0, count_connections_in(Config, VHost)),
 
-    set_vhost_connection_limit(Config, VHost, 0).
+    set_vhost_connection_limit(Config, VHost,  -1).
 
 cluster_single_vhost_limit2(Config) ->
     VHost = <<"/">>,
@@ -510,10 +573,10 @@ cluster_single_vhost_limit2(Config) ->
     Conn2 = open_unmanaged_connection(Config, 0, VHost),
 
     %% we've crossed the limit
-    {error, not_allowed} = open_unmanaged_connection(Config, 0, VHost),
+    expect_that_client_connection_is_rejected(Config, 0, VHost),
 
     timer:sleep(200),
-    {error, not_allowed} = open_unmanaged_connection(Config, 1, VHost),
+    expect_that_client_connection_is_rejected(Config, 1, VHost),
 
     set_vhost_connection_limit(Config, VHost, 5),
 
@@ -528,7 +591,7 @@ cluster_single_vhost_limit2(Config) ->
 
     ?assertEqual(0, count_connections_in(Config, VHost)),
 
-    set_vhost_connection_limit(Config, VHost, 0).
+    set_vhost_connection_limit(Config, VHost,  -1).
 
 single_node_vhost_deletion_forces_connection_closure(Config) ->
     VHost1 = <<"vhost1">>,
@@ -634,3 +697,12 @@ set_vhost_connection_limit(Config, NodeIndex, VHost, Count) ->
 
 await_running_node_refresh(_Config, _NodeIndex) ->
     timer:sleep(250).
+
+expect_that_client_connection_is_rejected(Config) ->
+    expect_that_client_connection_is_rejected(Config, 0).
+
+expect_that_client_connection_is_rejected(Config, NodeIndex) ->
+    {error, not_allowed} = open_unmanaged_connection(Config, NodeIndex).
+
+expect_that_client_connection_is_rejected(Config, NodeIndex, VHost) ->
+    {error, not_allowed} = open_unmanaged_connection(Config, NodeIndex, VHost).