Merge pull request #3319 from rabbitmq/mk-http-api-put-vhost-updates-tags-and-description

Make PUT /api/vhosts/{name} update tags and/or description for existing virtual hosts

Author: michaelklishin

deps/rabbit/src/rabbit_vhost.erl

@@ -203,6 +203,26 @@ do_add(Name, Description, Tags, ActingUser) ->
             {error, Msg}
     end.
 
+-spec update(vhost:name(), binary(), [atom()], rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
+update(Name, Description, Tags, ActingUser) ->
+    rabbit_misc:execute_mnesia_transaction(
+          fun () ->
+                  case mnesia:wread({rabbit_vhost, Name}) of
+                      [] ->
+                          {error, {no_such_vhost, Name}};
+                      [VHost0] ->
+                          VHost = vhost:merge_metadata(VHost0, #{description => Description, tags => Tags}),
+                          rabbit_log:debug("Updating a virtual host record ~p", [VHost]),
+                          ok = mnesia:write(rabbit_vhost, VHost, write),
+                          rabbit_event:notify(vhost_updated, info(VHost)
+                                ++ [{user_who_performed_action, ActingUser},
+                                    {description, Description},
+                                    {tags, Tags}]),
+                          ok
+                  end
+          end).
+
+
 -spec delete(vhost:name(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
 
 delete(VHost, ActingUser) ->
@@ -240,11 +260,12 @@ put_vhost(Name, Description, Tags0, Trace, Username) ->
       "null"      -> <<"">>;
       Other       -> Other
     end,
+    ParsedTags = parse_tags(Tags),
+    rabbit_log:debug("Parsed tags ~p to ~p", [Tags, ParsedTags]),
     Result = case exists(Name) of
-        true  -> ok;
+        true  ->
+            update(Name, Description, ParsedTags, Username);
         false ->
-            ParsedTags = parse_tags(Tags),
-            rabbit_log:debug("Parsed tags ~p to ~p", [Tags, ParsedTags]),
             add(Name, Description, ParsedTags, Username),
              %% wait for up to 45 seconds for the vhost to initialise
              %% on all nodes

deps/rabbit/src/vhost.erl

@@ -27,6 +27,7 @@
   get_tags/1,
   set_limits/2,
   set_metadata/2,
+  merge_metadata/2,
   is_tagged_with/2
 ]).
 
@@ -173,6 +174,7 @@ set_limits(VHost, Value) ->
         vhost_v1:set_limits(VHost, Value)
     end.
 
+-spec set_metadata(vhost(), metadata()) -> vhost().
 set_metadata(VHost, Value) ->
     case record_version_to_use() of
       ?record_version ->
@@ -182,6 +184,18 @@ set_metadata(VHost, Value) ->
         VHost
     end.
 
+-spec merge_metadata(vhost(), metadata()) -> vhost().
+merge_metadata(VHost, Value) ->
+    case record_version_to_use() of
+      ?record_version ->
+        Meta0 = get_metadata(VHost),
+        NewMeta = maps:merge(Meta0, Value),
+        VHost#vhost{metadata = NewMeta};
+      _ ->
+        %% the field is not available, so this is a no-op
+        VHost
+    end.
+
 -spec is_tagged_with(vhost:vhost(), atom()) -> boolean().
 is_tagged_with(VHost, Tag) ->
     lists:member(Tag, get_tags(VHost)).

deps/rabbitmq_management/test/rabbit_mgmt_only_http_SUITE.erl

@@ -261,6 +261,27 @@ vhosts_test(Config) ->
     ?assert(not maps:is_key(recv_oct, GetFirst)),
     ?assert(maps:is_key(cluster_state, GetFirst)),
 
+    %% PUT can update metadata (description, tags)
+    Desc0 = "desc 0",
+    Meta0 = [
+        {description, Desc0},
+        {tags,        "tag1,tag2"}
+    ],
+    http_put(Config, "/vhosts/myvhost", Meta0, {group, '2xx'}),
+    #{description := Desc1, tags := Tags1} = http_get(Config, "/vhosts/myvhost", ?OK),
+    ?assertEqual(Desc0, Desc1),
+    ?assertEqual([<<"tag1">>, <<"tag2">>], Tags1),
+
+    Desc2 = "desc 2",
+    Meta2 = [
+        {description, Desc2},
+        {tags,        "tag3"}
+    ],
+    http_put(Config, "/vhosts/myvhost", Meta2, {group, '2xx'}),
+    #{description := Desc3, tags := Tags3} = http_get(Config, "/vhosts/myvhost", ?OK),
+    ?assertEqual(Desc2, Desc3),
+    ?assertEqual([<<"tag3">>], Tags3),
+
     %% Check individually
     Get = http_get(Config, "/vhosts/%2F", ?OK),
     assert_item(#{name => <<"/">>}, Get),