Skip to content

Commit ca6b404

Browse files
BenAtAmazonBenAtAmazon
committed
AWS peer discovery: add multi-hostname path support
1 parent 1c264fa commit ca6b404

File tree

4 files changed

+266
-18
lines changed

4 files changed

+266
-18
lines changed

‎deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,35 @@ fun(Conf) ->
108108
Value -> rabbit_peer_discovery_util:as_list(Value)
109109
end
110110
end}.
111+
112+
%% hostname_paths (multiple paths support)
113+
114+
{mapping, "cluster_formation.aws.hostname_path.$number", "rabbit.cluster_formation.peer_discovery_aws.aws_hostname_paths", [
115+
{datatype, string}
116+
]}.
117+
118+
{translation, "rabbit.cluster_formation.peer_discovery_aws.aws_hostname_paths",
119+
fun(Conf) ->
120+
Settings = cuttlefish_variable:filter_by_prefix("cluster_formation.aws.hostname_path", Conf),
121+
%% Helper function for number validation
122+
IsNumberString = fun(Str) ->
123+
try
124+
_ = list_to_integer(Str),
125+
true
126+
catch
127+
_:_ -> false
128+
end
129+
end,
130+
%% Extract and sort numbered paths (hostname_path.1, hostname_path.2, etc.)
131+
NumberedPaths = [{list_to_integer(N), V} ||
132+
{["cluster_formation", "aws", "hostname_path", N], V} <- Settings,
133+
IsNumberString(N)],
134+
case NumberedPaths of
135+
[] -> cuttlefish:unset();
136+
_ ->
137+
SortedPaths = lists:sort(NumberedPaths),
138+
[rabbit_peer_discovery_util:as_list(V) || {_, V} <- SortedPaths]
139+
end
140+
end}.
141+
142+

‎deps/rabbitmq_peer_discovery_aws/src/rabbit_peer_discovery_aws.erl‎

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@
6262
env_variable = "AWS_HOSTNAME_PATH",
6363
default_value = ["privateDnsName"]
6464
},
65+
aws_hostname_paths => #peer_discovery_config_entry_meta{
66+
type = list,
67+
env_variable = "AWS_HOSTNAME_PATHS",
68+
default_value = undefined
69+
},
6570
aws_use_private_ip => #peer_discovery_config_entry_meta{
6671
type = atom,
6772
env_variable = "AWS_USE_PRIVATE_IP",
@@ -318,10 +323,9 @@ get_hostname_name_from_reservation_set([], Accum) -> Accum;
318323
get_hostname_name_from_reservation_set([{"item", RI}|T], Accum) ->
319324
InstancesSet = proplists:get_value("instancesSet", RI),
320325
Items = [Item || {"item", Item} <- InstancesSet],
321-
HostnamePath = get_hostname_path(),
322-
Hostnames = [get_hostname(HostnamePath, Item) || Item <- Items],
323-
Hostnames2 = [Name || Name <- Hostnames, Name =/= ""],
324-
get_hostname_name_from_reservation_set(T, Accum ++ Hostnames2).
326+
HostnamePaths = get_hostname_paths(),
327+
UniqueHostnames = extract_unique_hostnames(HostnamePaths, Items),
328+
get_hostname_name_from_reservation_set(T, Accum ++ UniqueHostnames).
325329

326330
get_hostname_names(Path) ->
327331
case rabbitmq_aws:api_get_request("ec2", Path) of
@@ -347,23 +351,66 @@ get_hostname_by_tags(Tags) ->
347351
Names
348352
end.
349353

350-
-spec get_hostname_path() -> path().
351-
get_hostname_path() ->
352-
UsePrivateIP = get_config_key(aws_use_private_ip, ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY)),
353-
HostnamePath = get_config_key(aws_hostname_path, ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY)),
354-
FinalPath = case HostnamePath of
355-
["privateDnsName"] when UsePrivateIP -> ["privateIpAddress"];
356-
P -> P
354+
355+
356+
-spec get_hostname_paths() -> [path()].
357+
get_hostname_paths() ->
358+
M = ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY),
359+
UsePrivateIP = get_config_key(aws_use_private_ip, M),
360+
361+
%% Get the raw paths first (without override)
362+
RawPaths = case get_config_key(aws_hostname_paths, M) of
363+
undefined ->
364+
%% Fall back to single path behavior
365+
SinglePath = get_single_hostname_path_raw(M),
366+
?LOG_DEBUG("AWS peer discovery using single hostname path"),
367+
[SinglePath];
368+
Paths when is_list(Paths) ->
369+
?LOG_DEBUG("AWS peer discovery using multiple hostname paths"),
370+
Paths;
371+
_Invalid ->
372+
%% Non-list configuration, fall back to single path
373+
SinglePath = get_single_hostname_path_raw(M),
374+
?LOG_DEBUG("AWS peer discovery falling back to single hostname path"),
375+
[SinglePath]
357376
end,
358-
?LOG_DEBUG("AWS peer discovery using hostname path: ~tp", [FinalPath]),
359-
FinalPath.
377+
378+
%% Apply use_private_ip override to all paths consistently
379+
FinalPaths = apply_private_ip_override(RawPaths, UsePrivateIP),
380+
?LOG_DEBUG("AWS peer discovery final hostname paths: ~tp", [FinalPaths]),
381+
FinalPaths.
382+
383+
384+
385+
-spec get_single_hostname_path_raw(map()) -> path().
386+
get_single_hostname_path_raw(ConfigMap) ->
387+
HostnamePath = get_config_key(aws_hostname_path, ConfigMap),
388+
case HostnamePath of
389+
P when P =/= undefined -> P;
390+
undefined -> ["privateDnsName"] %% Default fallback
391+
end.
392+
393+
-spec apply_private_ip_override([path()], boolean()) -> [path()].
394+
apply_private_ip_override(Paths, UsePrivateIP) ->
395+
lists:map(fun(Path) ->
396+
case Path of
397+
["privateDnsName"] when UsePrivateIP -> ["privateIpAddress"];
398+
P -> P
399+
end
400+
end, Paths).
360401

361402
-spec get_hostname(path(), props()) -> string().
403+
get_hostname([], _Props) ->
404+
""; %% Handle empty paths gracefully
362405
get_hostname(Path, Props) ->
363-
List = lists:foldl(fun get_value/2, Props, Path),
364-
case io_lib:latin1_char_list(List) of
365-
true -> List;
366-
_ -> ""
406+
try
407+
List = lists:foldl(fun get_value/2, Props, Path),
408+
case io_lib:latin1_char_list(List) of
409+
true -> List;
410+
_ -> ""
411+
end
412+
catch
413+
_:_ -> "" %% Handle any path traversal errors gracefully
367414
end.
368415

369416
-spec get_value(string()|integer(), props()) -> props().
@@ -413,3 +460,15 @@ get_tags() ->
413460
maps:from_list(Value);
414461
_ -> Tags
415462
end.
463+
464+
%% Helper functions for multiple hostname paths support
465+
466+
-spec extract_unique_hostnames([path()], [props()]) -> [string()].
467+
extract_unique_hostnames(Paths, Items) ->
468+
%% Extract all hostnames from all paths for all items
469+
AllHostnames = [get_hostname(Path, Item) || Path <- Paths, Item <- Items],
470+
%% Filter out empty hostnames and remove duplicates
471+
ValidHostnames = [Name || Name <- AllHostnames, Name =/= ""],
472+
lists:usort(ValidHostnames).
473+
474+

‎deps/rabbitmq_peer_discovery_aws/test/config_schema_SUITE_data/rabbitmq_peer_discovery_aws.snippets‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,35 @@
103103
]}
104104
]}
105105
], [rabbitmq_peer_discovery_aws]
106+
},
107+
{aws_hostname_paths_multiple,
108+
"cluster_formation.aws.hostname_path.1 = networkInterfaceSet,2,privateIpAddressesSet,1,privateDnsName
109+
cluster_formation.aws.hostname_path.2 = privateDnsName
110+
cluster_formation.aws.hostname_path.3 = privateIpAddress",
111+
[
112+
{rabbit, [
113+
{cluster_formation, [
114+
{peer_discovery_aws, [
115+
{aws_hostname_paths, [
116+
["networkInterfaceSet", 2, "privateIpAddressesSet", 1, "privateDnsName"],
117+
["privateDnsName"],
118+
["privateIpAddress"]
119+
]}
120+
]}
121+
]}
122+
]}
123+
], [rabbitmq_peer_discovery_aws]
124+
},
125+
{aws_hostname_paths_single_numbered,
126+
"cluster_formation.aws.hostname_path.1 = privateDnsName",
127+
[
128+
{rabbit, [
129+
{cluster_formation, [
130+
{peer_discovery_aws, [
131+
{aws_hostname_paths, [["privateDnsName"]]}
132+
]}
133+
]}
134+
]}
135+
], [rabbitmq_peer_discovery_aws]
106136
}
107137
].

‎deps/rabbitmq_peer_discovery_aws/test/unit_SUITE.erl‎

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ groups() ->
2525
get_hostname_name_from_reservation_set,
2626
registration_support,
2727
network_interface_sorting,
28-
private_ip_address_sorting
28+
private_ip_address_sorting,
29+
multiple_hostname_paths_extraction,
30+
hostname_paths_helper_functions
2931
]},
3032
{lock, [], [
3133
lock_single_node,
@@ -190,6 +192,130 @@ lock_local_node_not_discovered(_Config) ->
190192
Expectation = {error, "Local node " ++ atom_to_list(node()) ++ " is not part of discovered nodes [me@host]"},
191193
?assertEqual(Expectation, rabbit_peer_discovery_aws:lock([me@host])).
192194

195+
multiple_hostname_paths_extraction(_Config) ->
196+
ok = eunit:test({
197+
foreach,
198+
fun on_start/0,
199+
fun on_finish/1,
200+
[{"multiple paths extraction",
201+
fun() ->
202+
%% Set up multiple hostname paths
203+
os:putenv("AWS_HOSTNAME_PATHS", "privateDnsName;privateIpAddress"),
204+
application:set_env(rabbit, cluster_formation, [
205+
{peer_discovery_aws, [
206+
{aws_hostname_paths, [["privateDnsName"], ["privateIpAddress"]]}
207+
]}
208+
]),
209+
210+
%% Test that multiple hostnames are extracted
211+
Expected = ["ip-10-0-16-29.eu-west-1.compute.internal", "10.0.16.29",
212+
"ip-10-0-16-31.eu-west-1.compute.internal", "10.0.16.31"],
213+
Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(
214+
reservation_set(), []),
215+
?assertEqual(Expected, Result)
216+
end},
217+
{"single path fallback",
218+
fun() ->
219+
%% Test fallback to single path when no numbered paths configured
220+
Expected = ["ip-10-0-16-29.eu-west-1.compute.internal",
221+
"ip-10-0-16-31.eu-west-1.compute.internal"],
222+
Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(
223+
reservation_set(), []),
224+
?assertEqual(Expected, Result)
225+
end},
226+
{"use_private_ip override with multiple paths",
227+
fun() ->
228+
%% Test that use_private_ip still works with multiple paths
229+
os:putenv("AWS_USE_PRIVATE_IP", "true"),
230+
application:set_env(rabbit, cluster_formation, [
231+
{peer_discovery_aws, [
232+
{aws_hostname_paths, [["privateDnsName"], ["privateIpAddress"]]},
233+
{aws_use_private_ip, true}
234+
]}
235+
]),
236+
237+
%% Should get private IPs for privateDnsName path, plus privateIpAddress path
238+
Expected = ["10.0.16.29", "10.0.16.29", "10.0.16.31", "10.0.16.31"],
239+
Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(
240+
reservation_set(), []),
241+
%% After deduplication, should have unique IPs
242+
UniqueResult = rabbit_peer_discovery_aws:remove_duplicates_preserve_order(Result),
243+
ExpectedUnique = ["10.0.16.29", "10.0.16.31"],
244+
?assertEqual(ExpectedUnique, UniqueResult)
245+
end},
246+
{"no hostname paths configured - default fallback",
247+
fun() ->
248+
%% Test the scenario that was failing: no hostname paths configured at all
249+
%% This should fall back to default ["privateDnsName"] behavior
250+
%% Clear all hostname path configuration
251+
application:unset_env(rabbit, cluster_formation),
252+
253+
Expected = ["ip-10-0-16-29.eu-west-1.compute.internal",
254+
"ip-10-0-16-31.eu-west-1.compute.internal"],
255+
Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(
256+
reservation_set(), []),
257+
?assertEqual(Expected, Result)
258+
end},
259+
{"invalid hostname paths configuration - graceful degradation",
260+
fun() ->
261+
%% Test invalid configuration handling - should fall back to single path
262+
application:set_env(rabbit, cluster_formation, [
263+
{peer_discovery_aws, [
264+
{aws_hostname_paths, invalid_config}
265+
]}
266+
]),
267+
268+
%% Should fall back to single path behavior (same as no config)
269+
Expected = ["ip-10-0-16-29.eu-west-1.compute.internal",
270+
"ip-10-0-16-31.eu-west-1.compute.internal"],
271+
Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(
272+
reservation_set(), []),
273+
?assertEqual(Expected, Result)
274+
end},
275+
{"malformed paths in configuration - graceful degradation",
276+
fun() ->
277+
%% Test malformed paths - should extract what it can, ignore invalid ones
278+
application:set_env(rabbit, cluster_formation, [
279+
{peer_discovery_aws, [
280+
{aws_hostname_paths, [["privateDnsName"], [], [""], [invalid_atom]]}
281+
]}
282+
]),
283+
284+
%% Should extract hostnames from valid paths only
285+
%% Only ["privateDnsName"] is valid, others return empty strings and are filtered
286+
Expected = ["ip-10-0-16-29.eu-west-1.compute.internal",
287+
"ip-10-0-16-31.eu-west-1.compute.internal"],
288+
Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(
289+
reservation_set(), []),
290+
?assertEqual(Expected, Result)
291+
end}]
292+
}).
293+
294+
hostname_paths_helper_functions(_Config) ->
295+
%% Test extract_unique_hostnames
296+
Paths = [["privateDnsName"], ["privateIpAddress"]],
297+
Items = [proplists:get_value("item", Item) || {"item", Item} <-
298+
proplists:get_value("instancesSet",
299+
proplists:get_value("item", hd(reservation_set())))],
300+
301+
Hostnames = rabbit_peer_discovery_aws:extract_unique_hostnames(Paths, Items),
302+
?assertEqual(2, length(Hostnames)), % Should extract 2 unique hostnames
303+
304+
%% Test with duplicates - should be automatically removed
305+
DuplicatePaths = [["privateDnsName"], ["privateDnsName"], ["privateIpAddress"]],
306+
UniqueHostnames = rabbit_peer_discovery_aws:extract_unique_hostnames(DuplicatePaths, Items),
307+
?assertEqual(2, length(UniqueHostnames)), % Should still be 2 unique hostnames
308+
309+
%% Test empty path handling - should return empty string gracefully
310+
EmptyPath = [],
311+
EmptyResult = rabbit_peer_discovery_aws:get_hostname(EmptyPath, [{"test", "value"}]),
312+
?assertEqual("", EmptyResult),
313+
314+
%% Test invalid path handling - should return empty string gracefully
315+
InvalidPath = [invalid_atom],
316+
InvalidResult = rabbit_peer_discovery_aws:get_hostname(InvalidPath, [{"test", "value"}]),
317+
?assertEqual("", InvalidResult).
318+
193319
%%%
194320
%%% Implementation
195321
%%%
@@ -203,6 +329,7 @@ on_finish(_Config) ->
203329
reset() ->
204330
application:unset_env(rabbit, cluster_formation),
205331
os:unsetenv("AWS_HOSTNAME_PATH"),
332+
os:unsetenv("AWS_HOSTNAME_PATHS"),
206333
os:unsetenv("AWS_USE_PRIVATE_IP").
207334

208335
reservation_set() ->

0 commit comments

Comments
 (0)