Skip to content

Commit

Permalink
MB-51208: get_hostnames should not assume the whole cluster is EE
Browse files Browse the repository at this point in the history
The get_hostnames function checks if the cluster is EE, and if so
it builds nodenames using regular rest port and ssl port. But this
is incorrect, because the cluster can contain both EE and CE nodes
simultaneously. Any attempt to build a hostname with ssl port for a
CE node when executed in a EE node will crash.

Change-Id: I34603eb9d9789433529cbf57f216ef2b30a5cbb1
Reviewed-on: https://review.couchbase.org/c/ns_server/+/171613
Tested-by: Timofey Barmin <timofey.barmin@couchbase.com>
Reviewed-by: Steve Watanabe <steve.watanabe@couchbase.com>
  • Loading branch information
timofey-barmin committed Mar 2, 2022
1 parent db1b998 commit 43e8640
Showing 1 changed file with 17 additions and 12 deletions.
29 changes: 17 additions & 12 deletions src/menelaus_web_node.erl
Expand Up @@ -482,10 +482,12 @@ build_node_hostname(Config, Node, LocalAddr, Options) ->
true -> LocalAddr;
false -> H
end,
Port = proplists:get_value(port, Options, rest_port),
list_to_binary(
misc:join_host_port(Host,
service_ports:get_port(Port, Config, Node))).
PortName = proplists:get_value(port, Options, rest_port),
Port = case service_ports:get_port(PortName, Config, Node) of
undefined -> error(no_port);
P -> P
end,
list_to_binary(misc:join_host_port(Host, Port)).

alternate_addresses_json(Node, Config, Snapshot, WantedPorts) ->
{ExtHostname, ExtPorts} =
Expand Down Expand Up @@ -592,7 +594,16 @@ get_hostnames(Req, NodeStatus, Options) when is_atom(NodeStatus) ->
get_hostnames(Req, Nodes, Options) when is_list(Nodes) ->
Config = ns_config:get(),
LocalAddr = local_addr(Req),
[{N, build_node_hostname(Config, N, LocalAddr, Options)} || N <- Nodes].
lists:filtermap(
fun (N) ->
try build_node_hostname(Config, N, LocalAddr, Options) of
NodeHostname -> {true, {N, NodeHostname}}
catch
%% Might happen when the node is CE and we are searching by
%% ssl port
error:no_port -> false
end
end, Nodes).

%% Node list
%% GET /pools/default/buckets/{Id}/nodes
Expand Down Expand Up @@ -632,13 +643,7 @@ find_node_hostname(HostPortStr, Req, NodeStatus) ->
Normalized ->
HostPortBin = list_to_binary(Normalized),
NHs = get_hostnames(Req, NodeStatus) ++
case cluster_compat_mode:is_enterprise() of
true ->
get_hostnames(Req, NodeStatus,
[{port, ssl_rest_port}]);
false ->
[]
end,
get_hostnames(Req, NodeStatus, [{port, ssl_rest_port}]),
case [N || {N, CandidateHostPort} <- NHs,
CandidateHostPort =:= HostPortBin] of
[] ->
Expand Down

0 comments on commit 43e8640

Please sign in to comment.