Repository: rabbitmq/rabbitmq-server (language: javascript)

Issue:
Shovel HTTP API: GET /api/shovels/{vhost}/{name} incorrectly returns a list of shovels
See https://github.com/rabbitmq/rabbitmq-website/discussions/2018.

Its DELETE counterparts are not affected because they do not produce a response.


Discussion / hints:

Fix patch:
diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_parameters.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_parameters.erl
index fc3ab32b24b3..886e339952e3 100644
--- a/deps/rabbitmq_management/src/rabbit_mgmt_wm_parameters.erl
+++ b/deps/rabbitmq_management/src/rabbit_mgmt_wm_parameters.erl
@@ -43,8 +43,7 @@ is_authorized(ReqData, Context) ->
 %%--------------------------------------------------------------------
 
 %% Hackish fix to make sure we return a JSON object instead of an empty list
-%% when the publish-properties value is empty. Should be removed in 3.7.0
-%% when we switch to a new JSON library.
+%% when the publish-properties value is empty.
 fix_shovel_publish_properties(P) ->
     case lists:keyfind(component, 1, P) of
         {_, <<"shovel">>} ->
diff --git a/deps/rabbitmq_shovel_management/app.bzl b/deps/rabbitmq_shovel_management/app.bzl
index 0ca17b66892d..3c338cf4f318 100644
--- a/deps/rabbitmq_shovel_management/app.bzl
+++ b/deps/rabbitmq_shovel_management/app.bzl
@@ -9,7 +9,8 @@ def all_beam_files(name = "all_beam_files"):
     erlang_bytecode(
         name = "other_beam",
         srcs = [
-            "src/rabbit_shovel_mgmt.erl",
+            "src/rabbit_shovel_mgmt_shovel.erl",
+            "src/rabbit_shovel_mgmt_shovels.erl",
             "src/rabbit_shovel_mgmt_util.erl",
         ],
         hdrs = [":public_and_private_hdrs"],
@@ -33,7 +34,8 @@ def all_test_beam_files(name = "all_test_beam_files"):
         name = "test_other_beam",
         testonly = True,
         srcs = [
-            "src/rabbit_shovel_mgmt.erl",
+            "src/rabbit_shovel_mgmt_shovel.erl",
+            "src/rabbit_shovel_mgmt_shovels.erl",
             "src/rabbit_shovel_mgmt_util.erl",
         ],
         hdrs = [":public_and_private_hdrs"],
@@ -72,7 +74,8 @@ def all_srcs(name = "all_srcs"):
     filegroup(
         name = "srcs",
         srcs = [
-            "src/rabbit_shovel_mgmt.erl",
+            "src/rabbit_shovel_mgmt_shovel.erl",
+            "src/rabbit_shovel_mgmt_shovels.erl",
             "src/rabbit_shovel_mgmt_util.erl",
         ],
     )
diff --git a/deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt.erl b/deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt_shovel.erl
similarity index 91%
rename from deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt.erl
rename to deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt_shovel.erl
index 2c414bded340..d52022d05dda 100644
--- a/deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt.erl
+++ b/deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt_shovel.erl
@@ -5,7 +5,7 @@
 %% Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
 %%
 
--module(rabbit_shovel_mgmt).
+-module(rabbit_shovel_mgmt_shovel).
 
 -behaviour(rabbit_mgmt_extension).
 
@@ -19,9 +19,9 @@
 -include_lib("amqp_client/include/amqp_client.hrl").
 -include("rabbit_shovel_mgmt.hrl").
 
-dispatcher() -> [{"/shovels",        ?MODULE, []},
-                 {"/shovels/:vhost", ?MODULE, []},
-                 {"/shovels/vhost/:vhost/:name", ?MODULE, []},
+-define(COMPONENT, <<"shovel">>).
+
+dispatcher() -> [{"/shovels/vhost/:vhost/:name", ?MODULE, []},
                  {"/shovels/vhost/:vhost/:name/restart", ?MODULE, []}].
 
 web_ui()     -> [{javascript, <<"shovel.js">>}].
@@ -42,7 +42,7 @@ resource_exists(ReqData, Context) ->
                 not_found ->
                     false;
                 VHost ->
-                    case rabbit_mgmt_util:id(name, ReqData) of
+                    case name(ReqData) of
                         none -> true;
                         Name ->
                             %% Deleting or restarting a shovel
@@ -65,8 +65,10 @@ resource_exists(ReqData, Context) ->
     {Reply, ReqData, Context}.
 
 to_json(ReqData, Context) ->
-    rabbit_mgmt_util:reply_list(
-      filter_vhost_req(rabbit_shovel_mgmt_util:status(ReqData, Context), ReqData), ReqData, Context).
+    Shovel = parameter(ReqData),
+    rabbit_mgmt_util:reply(rabbit_mgmt_format:parameter(
+        rabbit_mgmt_wm_parameters:fix_shovel_publish_properties(Shovel)),
+        ReqData, Context).
 
 is_authorized(ReqData, Context) ->
     rabbit_mgmt_util:is_authorized_monitor(ReqData, Context).
@@ -115,6 +117,19 @@ delete_resource(ReqData, #context{user = #user{username = Username}}=Context) ->
 
 %%--------------------------------------------------------------------
 
+name(ReqData) -> rabbit_mgmt_util:id(name, ReqData).
+
+parameter(ReqData) ->
+    VHostName = rabbit_mgmt_util:vhost(ReqData),
+    Name = name(ReqData),
+    if
+        VHostName =/= not_found andalso
+            Name =/= none ->
+            rabbit_runtime_parameters:lookup(VHostName, ?COMPONENT, Name);
+        true ->
+            not_found
+    end.
+
 is_restart(ReqData) ->
     Path = cowboy_req:path(ReqData),
     case string:find(Path, "/restart", trailing) of
@@ -122,13 +137,6 @@ is_restart(ReqData) ->
         _ -> true
     end.
 
-filter_vhost_req(List, ReqData) ->
-    case rabbit_mgmt_util:vhost(ReqData) of
-        none      -> List;
-        VHost     -> [I || I <- List,
-                           pget(vhost, I) =:= VHost]
-    end.
-
 get_shovel_node(VHost, Name, ReqData, Context) ->
     Shovels = rabbit_shovel_mgmt_util:status(ReqData, Context),
     Match   = find_matching_shovel(VHost, Name, Shovels),
diff --git a/deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt_shovels.erl b/deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt_shovels.erl
new file mode 100644
index 000000000000..ca5a5f528556
--- /dev/null
+++ b/deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt_shovels.erl
@@ -0,0 +1,57 @@
+%% 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) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
+%%
+
+-module(rabbit_shovel_mgmt_shovels).
+
+-behaviour(rabbit_mgmt_extension).
+
+-export([dispatcher/0, web_ui/0]).
+-export([init/2, to_json/2, resource_exists/2, content_types_provided/2,
+         is_authorized/2, allowed_methods/2]).
+
+-import(rabbit_misc, [pget/2]).
+
+-include_lib("rabbitmq_management_agent/include/rabbit_mgmt_records.hrl").
+-include_lib("amqp_client/include/amqp_client.hrl").
+-include("rabbit_shovel_mgmt.hrl").
+
+dispatcher() -> [{"/shovels",        ?MODULE, []},
+                 {"/shovels/:vhost", ?MODULE, []}].
+
+web_ui()     -> [{javascript, <<"shovel.js">>}].
+
+%%--------------------------------------------------------------------
+
+init(Req, _Opts) ->
+    {cowboy_rest, rabbit_mgmt_cors:set_headers(Req, ?MODULE), #context{}}.
+
+content_types_provided(ReqData, Context) ->
+   {[{<<"application/json">>, to_json}], ReqData, Context}.
+
+allowed_methods(ReqData, Context) ->
+    {[<<"HEAD">>, <<"GET">>, <<"OPTIONS">>], ReqData, Context}.
+
+resource_exists(ReqData, Context) ->
+    Reply = case rabbit_mgmt_util:vhost(ReqData) of
+                not_found -> false;
+                _Found -> true
+            end,
+    {Reply, ReqData, Context}.
+
+to_json(ReqData, Context) ->
+    rabbit_mgmt_util:reply_list(
+      filter_vhost_req(rabbit_shovel_mgmt_util:status(ReqData, Context), ReqData), ReqData, Context).
+
+is_authorized(ReqData, Context) ->
+    rabbit_mgmt_util:is_authorized_monitor(ReqData, Context).
+
+filter_vhost_req(List, ReqData) ->
+    case rabbit_mgmt_util:vhost(ReqData) of
+        none      -> List;
+        VHost     -> [I || I <- List,
+                           pget(vhost, I) =:= VHost]
+    end.
diff --git a/moduleindex.yaml b/moduleindex.yaml
index 44d68673640b..481cb2878d61 100755
--- a/moduleindex.yaml
+++ b/moduleindex.yaml
@@ -1164,7 +1164,8 @@ rabbitmq_shovel:
 - rabbit_shovel_worker
 - rabbit_shovel_worker_sup
 rabbitmq_shovel_management:
-- rabbit_shovel_mgmt
+- rabbit_shovel_mgmt_shovel
+- rabbit_shovel_mgmt_shovels
 - rabbit_shovel_mgmt_util
 rabbitmq_stomp:
 - Elixir.RabbitMQ.CLI.Ctl.Commands.ListStompConnectionsCommand


Test patch:
diff --git a/deps/rabbitmq_shovel_management/test/http_SUITE.erl b/deps/rabbitmq_shovel_management/test/http_SUITE.erl
index 07d294086a5f..af1f02404bb7 100644
--- a/deps/rabbitmq_shovel_management/test/http_SUITE.erl
+++ b/deps/rabbitmq_shovel_management/test/http_SUITE.erl
@@ -8,6 +8,7 @@
 -module(http_SUITE).
 
 -include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
 -include_lib("rabbit_common/include/rabbit_framing.hrl").
 -include_lib("rabbitmq_ct_helpers/include/rabbit_mgmt_test.hrl").
 
@@ -27,6 +28,7 @@ groups() ->
     [
      {dynamic_shovels, [], [
                   start_and_list_a_dynamic_amqp10_shovel,
+                  start_and_get_a_dynamic_amqp10_shovel,
                   create_and_delete_a_dynamic_shovel_that_successfully_connects,
                   create_and_delete_a_dynamic_shovel_that_fails_to_connect
                  ]},
@@ -124,25 +126,33 @@ start_inets(Config) ->
 %% -------------------------------------------------------------------
 
 start_and_list_a_dynamic_amqp10_shovel(Config) ->
-    Port = integer_to_binary(
-             rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_amqp)),
-
     remove_all_dynamic_shovels(Config, <<"/">>),
-    ID = {<<"/">>, <<"dynamic-amqp10-1">>},
+    Name = <<"dynamic-amqp10-await-startup-1">>,
+    ID = {<<"/">>, Name},
     await_shovel_removed(Config, ID),
 
-    http_put(Config, "/parameters/shovel/%2f/dynamic-amqp10-1",
-                  #{value => #{'src-protocol' => <<"amqp10">>,
-                               'src-uri' => <<"amqp://localhost:", Port/binary>>,
-                               'src-address'  => <<"test">>,
-                               'dest-protocol' => <<"amqp10">>,
-                               'dest-uri' => <<"amqp://localhost:", Port/binary>>,
-                               'dest-address' => <<"test2">>,
-                               'dest-properties' => #{},
-                               'dest-application-properties' => #{},
-                               'dest-message-annotations' => #{}}}, ?CREATED),
+    declare_shovel(Config, Name),
+    await_shovel_startup(Config, ID),
+    Shovels = list_shovels(Config),
+    ?assert(lists:any(
+        fun(M) ->
+            maps:get(name, M) =:= Name
+        end, Shovels)),
+    delete_shovel(Config, <<"dynamic-amqp10-await-startup-1">>),
+
+    ok.
+
+start_and_get_a_dynamic_amqp10_shovel(Config) ->
+    remove_all_dynamic_shovels(Config, <<"/">>),
+    Name = <<"dynamic-amqp10-get-shovel-1">>,
+    ID = {<<"/">>, Name},
+    await_shovel_removed(Config, ID),
 
+    declare_shovel(Config, Name),
     await_shovel_startup(Config, ID),
+    Sh = get_shovel(Config, Name),
+    ?assertEqual(Name, maps:get(name, Sh)),
+    delete_shovel(Config, <<"dynamic-amqp10-await-startup-1">>),
 
     ok.
 
@@ -317,14 +327,48 @@ assert_item(ExpI, ActI) ->
     ExpI = maps:with(maps:keys(ExpI), ActI),
     ok.
 
+list_shovels(Config) ->
+    list_shovels(Config, "%2F").
+
+list_shovels(Config, VirtualHost) ->
+    Path = io_lib:format("/shovels/~s", [VirtualHost]),
+    http_get(Config, Path, ?OK).
+
+get_shovel(Config, Name) ->
+    get_shovel(Config, "%2F", Name).
+
+get_shovel(Config, VirtualHost, Name) ->
+    Path = io_lib:format("/shovels/vhost/~s/~s", [VirtualHost, Name]),
+    http_get(Config, Path, ?OK).
+
 delete_shovel(Config, Name) ->
-    Path = io_lib:format("/shovels/vhost/%2F/~s", [Name]),
+    delete_shovel(Config, "%2F", Name).
+
+delete_shovel(Config, VirtualHost, Name) ->
+    Path = io_lib:format("/shovels/vhost/~s/~s", [VirtualHost, Name]),
     http_delete(Config, Path, ?NO_CONTENT).
 
 remove_all_dynamic_shovels(Config, VHost) ->
     rabbit_ct_broker_helpers:rpc(Config, 0,
         rabbit_runtime_parameters, clear_vhost, [VHost, <<"CT tests">>]).
 
+declare_shovel(Config, Name) ->
+    Port = integer_to_binary(
+        rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_amqp)),
+    http_put(Config, io_lib:format("/parameters/shovel/%2f/~ts", [Name]),
+        #{
+            value => #{
+                'src-protocol' => <<"amqp10">>,
+                'src-uri' => <<"amqp://localhost:", Port/binary>>,
+                'src-address'  => <<"test">>,
+                'dest-protocol' => <<"amqp10">>,
+                'dest-uri' => <<"amqp://localhost:", Port/binary>>,
+                'dest-address' => <<"test2">>,
+                'dest-properties' => #{},
+                'dest-application-properties' => #{},
+                'dest-message-annotations' => #{}}
+        }, ?CREATED).
+
 await_shovel_startup(Config, Name) ->
     await_shovel_startup(Config, Name, 10_000).