From c0c79bfd6324e61d99bb830826788598a6ad1afc Mon Sep 17 00:00:00 2001 From: Ben Nguyen Date: Tue, 7 Oct 2025 03:22:04 -0700 Subject: [PATCH 1/2] AWS peer discovery: add multi-hostname path support Adds support for multiple hostname paths in the AWS peer discovery plugin to enable zero-downtime rolling upgrades during hostname migration scenarios. The implementation allows RabbitMQ nodes to discover peers using multiple hostname paths, ensuring cluster formation succeeds even when nodes are configured with different hostname paths during rolling upgrades. Example usage: cluster_formation.aws.hostname_path.1 = privateDnsName cluster_formation.aws.hostname_path.2 = privateIpAddress Move tests that need per-test init and end to dedicated file. --- .../schema/rabbitmq_peer_discovery_aws.schema | 30 ++ .../src/rabbit_peer_discovery_aws.erl | 99 ++++- .../rabbitmq_peer_discovery_aws.snippets | 30 ++ .../test/unit_SUITE.erl | 146 +------ .../test/unit_reservation_set_SUITE.erl | 384 ++++++++++++++++++ 5 files changed, 525 insertions(+), 164 deletions(-) create mode 100644 deps/rabbitmq_peer_discovery_aws/test/unit_reservation_set_SUITE.erl diff --git a/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema b/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema index e3e47f011401..1f46ccfde1be 100644 --- a/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema +++ b/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema @@ -108,3 +108,33 @@ fun(Conf) -> Value -> rabbit_peer_discovery_util:as_list(Value) end end}. + +%% hostname_paths (multiple paths support) + +{mapping, "cluster_formation.aws.hostname_path.$number", "rabbit.cluster_formation.peer_discovery_aws.aws_hostname_paths", [ + {datatype, string} +]}. + +{translation, "rabbit.cluster_formation.peer_discovery_aws.aws_hostname_paths", +fun(Conf) -> + Settings = cuttlefish_variable:filter_by_prefix("cluster_formation.aws.hostname_path", Conf), + %% Helper function for number validation + IsNumberString = fun(Str) -> + case string:to_integer(Str) of + {_, ""} -> true; + _ -> false + end + end, + %% Extract and sort numbered paths (hostname_path.1, hostname_path.2, etc.) + NumberedPaths = [{list_to_integer(N), V} || + {["cluster_formation", "aws", "hostname_path", N], V} <- Settings, + IsNumberString(N)], + case NumberedPaths of + [] -> cuttlefish:unset(); + _ -> + SortedPaths = lists:sort(NumberedPaths), + [rabbit_peer_discovery_util:as_list(V) || {_, V} <- SortedPaths] + end +end}. + + diff --git a/deps/rabbitmq_peer_discovery_aws/src/rabbit_peer_discovery_aws.erl b/deps/rabbitmq_peer_discovery_aws/src/rabbit_peer_discovery_aws.erl index c4c0da98a2a8..ece38fc41f7e 100644 --- a/deps/rabbitmq_peer_discovery_aws/src/rabbit_peer_discovery_aws.erl +++ b/deps/rabbitmq_peer_discovery_aws/src/rabbit_peer_discovery_aws.erl @@ -62,6 +62,11 @@ env_variable = "AWS_HOSTNAME_PATH", default_value = ["privateDnsName"] }, + aws_hostname_paths => #peer_discovery_config_entry_meta{ + type = list, + env_variable = "AWS_HOSTNAME_PATHS", + default_value = [] + }, aws_use_private_ip => #peer_discovery_config_entry_meta{ type = atom, env_variable = "AWS_USE_PRIVATE_IP", @@ -318,10 +323,11 @@ get_hostname_name_from_reservation_set([], Accum) -> Accum; get_hostname_name_from_reservation_set([{"item", RI}|T], Accum) -> InstancesSet = proplists:get_value("instancesSet", RI), Items = [Item || {"item", Item} <- InstancesSet], - HostnamePath = get_hostname_path(), - Hostnames = [get_hostname(HostnamePath, Item) || Item <- Items], - Hostnames2 = [Name || Name <- Hostnames, Name =/= ""], - get_hostname_name_from_reservation_set(T, Accum ++ Hostnames2). + HostnamePaths = get_hostname_paths(), + ?LOG_DEBUG("AWS peer discovery: processing reservation with ~p instances using hostname paths: ~tp", + [length(Items), HostnamePaths]), + UniqueHostnames = extract_unique_hostnames(HostnamePaths, Items), + get_hostname_name_from_reservation_set(T, Accum ++ UniqueHostnames). get_hostname_names(Path) -> case rabbitmq_aws:api_get_request("ec2", Path) of @@ -347,31 +353,69 @@ get_hostname_by_tags(Tags) -> Names end. --spec get_hostname_path() -> path(). -get_hostname_path() -> - UsePrivateIP = get_config_key(aws_use_private_ip, ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY)), - HostnamePath = get_config_key(aws_hostname_path, ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY)), - FinalPath = case HostnamePath of - ["privateDnsName"] when UsePrivateIP -> ["privateIpAddress"]; - P -> P +-spec get_hostname_paths() -> [path()]. +get_hostname_paths() -> + M = ?CONFIG_MODULE:config_map(?BACKEND_CONFIG_KEY), + UsePrivateIP = get_config_key(aws_use_private_ip, M), + RawPaths = case get_config_key(aws_hostname_paths, M) of + Paths when is_list(Paths), Paths =/= [] -> + ?LOG_DEBUG("AWS peer discovery using multiple hostname paths"), + Paths; + _ -> + %% Use single path configuration (including when Paths is []) + SinglePath = get_single_hostname_path_raw(M), + ?LOG_DEBUG("AWS peer discovery using single hostname path"), + [SinglePath] end, - ?LOG_DEBUG("AWS peer discovery using hostname path: ~tp", [FinalPath]), - FinalPath. + %% Apply use_private_ip override to all paths consistently + FinalPaths = apply_private_ip_override(RawPaths, UsePrivateIP), + ?LOG_DEBUG("AWS peer discovery final hostname paths: ~tp", [FinalPaths]), + FinalPaths. + +-spec get_single_hostname_path_raw(map()) -> path(). +get_single_hostname_path_raw(ConfigMap) -> + case get_config_key(aws_hostname_path, ConfigMap) of + undefined -> + ["privateDnsName"]; + P -> + P + end. + +-spec apply_private_ip_override([path()], boolean()) -> [path()]. +apply_private_ip_override(Paths, UsePrivateIP) -> + apply_private_ip_override(Paths, UsePrivateIP, []). +apply_private_ip_override([], _, Acc) -> + lists:reverse(Acc); +apply_private_ip_override([["privateDnsName"] | Paths], true, Acc0) -> + Acc1 = [["privateIpAddress"] | Acc0], + apply_private_ip_override(Paths, true, Acc1); +apply_private_ip_override([Path | Paths], UsePrivateIP, Acc0) -> + Acc1 = [Path | Acc0], + apply_private_ip_override(Paths, UsePrivateIP, Acc1). -spec get_hostname(path(), props()) -> string(). +get_hostname([], _Props) -> + ?LOG_DEBUG("AWS peer discovery: empty hostname path provided"), + ""; %% Handle empty paths gracefully get_hostname(Path, Props) -> List = lists:foldl(fun get_value/2, Props, Path), case io_lib:latin1_char_list(List) of - true -> List; - _ -> "" + true -> + ?LOG_DEBUG("AWS peer discovery: extracted hostname '~ts' from path ~tp", [List, Path]), + List; + _ -> + ?LOG_DEBUG("AWS peer discovery: invalid hostname format from path ~tp, result: ~tp", [Path, List]), + "" end. -spec get_value(string()|integer(), props()) -> props(). -get_value(_, []) -> - []; -get_value(Key, Props) when is_integer(Key) -> - {"item", Props2} = lists:nth(Key, Props), - Props2; +get_value(Key, Props) when is_integer(Key), is_list(Props), length(Props) >= Key, Key > 0 -> + case lists:nth(Key, Props) of + {"item", Props2} -> Props2; + _ -> [] % Malformed data + end; +get_value(Key, _Props) when is_integer(Key) -> + []; % Out of bounds or empty list get_value(Key, Props) -> Value = proplists:get_value(Key, Props), sort_ec2_hostname_path_set_members(Key, Value). @@ -413,3 +457,18 @@ get_tags() -> maps:from_list(Value); _ -> Tags end. + +%% Helper functions for multiple hostname paths support + +-spec extract_unique_hostnames([path()], [props()]) -> [string()]. +extract_unique_hostnames(Paths, Items) -> + ?LOG_DEBUG("AWS peer discovery: extracting hostnames using ~p paths for ~p items", + [length(Paths), length(Items)]), + %% Extract all hostnames from all paths for all items + AllHostnames = [get_hostname(Path, Item) || Path <- Paths, Item <- Items], + %% Filter out empty hostnames and remove duplicates + ValidHostnames = [Name || Name <- AllHostnames, Name =/= ""], + UniqueHostnames = lists:uniq(ValidHostnames), + ?LOG_DEBUG("AWS peer discovery: extracted ~p total hostnames, ~p valid, ~p unique: ~tp", + [length(AllHostnames), length(ValidHostnames), length(UniqueHostnames), UniqueHostnames]), + UniqueHostnames. diff --git a/deps/rabbitmq_peer_discovery_aws/test/config_schema_SUITE_data/rabbitmq_peer_discovery_aws.snippets b/deps/rabbitmq_peer_discovery_aws/test/config_schema_SUITE_data/rabbitmq_peer_discovery_aws.snippets index c9ac1c9ad316..ff53bf6f00a6 100644 --- a/deps/rabbitmq_peer_discovery_aws/test/config_schema_SUITE_data/rabbitmq_peer_discovery_aws.snippets +++ b/deps/rabbitmq_peer_discovery_aws/test/config_schema_SUITE_data/rabbitmq_peer_discovery_aws.snippets @@ -103,5 +103,35 @@ ]} ]} ], [rabbitmq_peer_discovery_aws] + }, + {aws_hostname_paths_multiple, + "cluster_formation.aws.hostname_path.1 = networkInterfaceSet,2,privateIpAddressesSet,1,privateDnsName + cluster_formation.aws.hostname_path.2 = privateDnsName + cluster_formation.aws.hostname_path.3 = privateIpAddress", + [ + {rabbit, [ + {cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_paths, [ + ["networkInterfaceSet", 2, "privateIpAddressesSet", 1, "privateDnsName"], + ["privateDnsName"], + ["privateIpAddress"] + ]} + ]} + ]} + ]} + ], [rabbitmq_peer_discovery_aws] + }, + {aws_hostname_paths_single_numbered, + "cluster_formation.aws.hostname_path.1 = privateDnsName", + [ + {rabbit, [ + {cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_paths, [["privateDnsName"]]} + ]} + ]} + ]} + ], [rabbitmq_peer_discovery_aws] } ]. diff --git a/deps/rabbitmq_peer_discovery_aws/test/unit_SUITE.erl b/deps/rabbitmq_peer_discovery_aws/test/unit_SUITE.erl index c705ef3853d1..d5bf461a2280 100644 --- a/deps/rabbitmq_peer_discovery_aws/test/unit_SUITE.erl +++ b/deps/rabbitmq_peer_discovery_aws/test/unit_SUITE.erl @@ -8,10 +8,10 @@ -module(unit_SUITE). -compile(export_all). + -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). - all() -> [ {group, unit}, @@ -22,7 +22,6 @@ groups() -> [ {unit, [], [ maybe_add_tag_filters, - get_hostname_name_from_reservation_set, registration_support, network_interface_sorting, private_ip_address_sorting @@ -46,48 +45,7 @@ maybe_add_tag_filters(_Config) -> {"Filter.1.Name", "tag:region"}, {"Filter.1.Value.1", "us-west-2"}]), Result = lists:sort(rabbit_peer_discovery_aws:maybe_add_tag_filters(Tags, [], 1)), - ?assertEqual(Expectation, Result). - -get_hostname_name_from_reservation_set(_Config) -> - ok = eunit:test({ - foreach, - fun on_start/0, - fun on_finish/1, - [{"from private DNS", - fun() -> - Expectation = ["ip-10-0-16-29.eu-west-1.compute.internal", - "ip-10-0-16-31.eu-west-1.compute.internal"], - ?assertEqual(Expectation, - rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( - reservation_set(), [])) - end}, - {"from arbitrary path", - fun() -> - os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,1,association,publicDnsName"), - Expectation = ["ec2-203-0-113-11.eu-west-1.compute.amazonaws.com", - "ec2-203-0-113-21.eu-west-1.compute.amazonaws.com"], - ?assertEqual(Expectation, - rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( - reservation_set(), [])) - end}, - {"from private IP", - fun() -> - os:putenv("AWS_USE_PRIVATE_IP", "true"), - Expectation = ["10.0.16.29", "10.0.16.31"], - ?assertEqual(Expectation, - rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( - reservation_set(), [])) - end}, - {"from private IP DNS in network interface", - fun() -> - os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,2,privateIpAddressesSet,1,privateDnsName"), - Expectation = ["ip-10-0-15-100.eu-west-1.compute.internal", - "ip-10-0-16-31.eu-west-1.compute.internal"], - ?assertEqual(Expectation, - rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( - reservation_set(), [])) - end}] - }). + ?assertMatch(Expectation, Result). registration_support(_Config) -> ?assertEqual(false, rabbit_peer_discovery_aws:supports_registration()). @@ -189,103 +147,3 @@ lock_multiple_nodes(_Config) -> lock_local_node_not_discovered(_Config) -> Expectation = {error, "Local node " ++ atom_to_list(node()) ++ " is not part of discovered nodes [me@host]"}, ?assertEqual(Expectation, rabbit_peer_discovery_aws:lock([me@host])). - -%%% -%%% Implementation -%%% - -on_start() -> - reset(). - -on_finish(_Config) -> - reset(). - -reset() -> - application:unset_env(rabbit, cluster_formation), - os:unsetenv("AWS_HOSTNAME_PATH"), - os:unsetenv("AWS_USE_PRIVATE_IP"). - -reservation_set() -> - [{"item", [{"reservationId","r-006cfdbf8d04c5f01"}, - {"ownerId","248536293561"}, - {"groupSet",[]}, - {"instancesSet", - [{"item", - [{"instanceId","i-0c6d048641f09cad2"}, - {"imageId","ami-ef4c7989"}, - {"instanceState", - [{"code","16"},{"name","running"}]}, - {"privateDnsName", - "ip-10-0-16-29.eu-west-1.compute.internal"}, - {"dnsName",[]}, - {"instanceType","c4.large"}, - {"launchTime","2017-04-07T12:05:10"}, - {"subnetId","subnet-61ff660"}, - {"vpcId","vpc-4fe1562b"}, - {"networkInterfaceSet", [ - {"item", - [{"attachment", [{"deviceIndex", "1"}]}, - {"association", - [{"publicIp","203.0.113.12"}, - {"publicDnsName", - "ec2-203-0-113-12.eu-west-1.compute.amazonaws.com"}, - {"ipOwnerId","amazon"}]}, - {"privateIpAddressesSet", [ - {"item", [ - {"privateIpAddress", "10.0.15.101"}, - {"privateDnsName", "ip-10-0-15-101.eu-west-1.compute.internal"}, - {"primary", "false"} - ]}, - {"item", [ - {"privateIpAddress", "10.0.15.100"}, - {"privateDnsName", "ip-10-0-15-100.eu-west-1.compute.internal"}, - {"primary", "true"} - ]} - ]}]}, - {"item", - [{"attachment", [{"deviceIndex", "0"}]}, - {"association", - [{"publicIp","203.0.113.11"}, - {"publicDnsName", - "ec2-203-0-113-11.eu-west-1.compute.amazonaws.com"}, - {"ipOwnerId","amazon"}]}]}]}, - {"privateIpAddress","10.0.16.29"}]}]}]}, - {"item", [{"reservationId","r-006cfdbf8d04c5f01"}, - {"ownerId","248536293561"}, - {"groupSet",[]}, - {"instancesSet", - [{"item", - [{"instanceId","i-1c6d048641f09cad2"}, - {"imageId","ami-af4c7989"}, - {"instanceState", - [{"code","16"},{"name","running"}]}, - {"privateDnsName", - "ip-10-0-16-31.eu-west-1.compute.internal"}, - {"dnsName",[]}, - {"instanceType","c4.large"}, - {"launchTime","2017-04-07T12:05:10"}, - {"subnetId","subnet-61ff660"}, - {"vpcId","vpc-4fe1562b"}, - {"networkInterfaceSet", [ - {"item", - [{"attachment", [{"deviceIndex", "0"}]}, - {"association", - [{"publicIp","203.0.113.21"}, - {"publicDnsName", - "ec2-203-0-113-21.eu-west-1.compute.amazonaws.com"}, - {"ipOwnerId","amazon"}]}]}, - {"item", - [{"attachment", [{"deviceIndex", "1"}]}, - {"association", - [{"publicIp","203.0.113.22"}, - {"publicDnsName", - "ec2-203-0-113-22.eu-west-1.compute.amazonaws.com"}, - {"ipOwnerId","amazon"}]}, - {"privateIpAddressesSet", [ - {"item", [ - {"privateIpAddress", "10.0.16.31"}, - {"privateDnsName", "ip-10-0-16-31.eu-west-1.compute.internal"}, - {"primary", "true"} - ]} - ]}]}]}, - {"privateIpAddress","10.0.16.31"}]}]}]}]. diff --git a/deps/rabbitmq_peer_discovery_aws/test/unit_reservation_set_SUITE.erl b/deps/rabbitmq_peer_discovery_aws/test/unit_reservation_set_SUITE.erl new file mode 100644 index 000000000000..24e0abf79887 --- /dev/null +++ b/deps/rabbitmq_peer_discovery_aws/test/unit_reservation_set_SUITE.erl @@ -0,0 +1,384 @@ +%% 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-2025 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. +%% + +-module(unit_reservation_set_SUITE). + +-compile(export_all). + +-include_lib("common_test/include/ct.hrl"). +-include_lib("eunit/include/eunit.hrl"). + +all() -> + [ + get_hostname_name_from_private_DNS, + get_hostname_name_from_arbitrary_path, + get_hostname_name_from_private_IP, + get_hostname_name_from_private_IP_DNS_in_network_interface, + multiple_paths_extraction, + single_path_fallback, + use_private_ip_override_with_multiple_paths, + complex_path_extraction, + hostname_paths_takes_precedence_over_hostname_path, + empty_hostname_paths_falls_back_to_hostname_path, + use_private_ip_with_both_hostname_path_and_hostname_paths, + use_private_ip_with_fallback_to_hostname_path, + out_of_bounds_index_should_not_crash, + nested_out_of_bounds_should_not_crash, + zero_index_should_not_crash, + multiple_paths_with_graceful_degradation, + empty_network_interface_set_should_not_crash, + hostname_paths_helper_functions + ]. + +init_per_testcase(_TestCase, Config) -> + ok = reset(), + Config. + +end_per_testcase(_TestCase, _Config) -> + ok = reset(). + +%%% +%%% Testcases +%%% + +get_hostname_name_from_private_DNS(_Config) -> + Expectation = ["ip-10-0-16-29.eu-west-1.compute.internal", + "ip-10-0-16-31.eu-west-1.compute.internal"], + ?assertEqual(Expectation, + rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), [])). + +get_hostname_name_from_arbitrary_path(_Config) -> + os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,1,association,publicDnsName"), + Expectation = ["ec2-203-0-113-11.eu-west-1.compute.amazonaws.com", + "ec2-203-0-113-21.eu-west-1.compute.amazonaws.com"], + ?assertMatch(Expectation, + rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), [])). + +get_hostname_name_from_private_IP(_Config) -> + os:putenv("AWS_USE_PRIVATE_IP", "true"), + Expectation = ["10.0.16.29", "10.0.16.31"], + ?assertMatch(Expectation, + rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), [])). + +get_hostname_name_from_private_IP_DNS_in_network_interface(_Config) -> + os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,2,privateIpAddressesSet,1,privateDnsName"), + Expectation = ["ip-10-0-15-100.eu-west-1.compute.internal", + "ip-10-0-16-31.eu-west-1.compute.internal"], + ?assertMatch(Expectation, + rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), [])). + +multiple_paths_extraction(_Config) -> + %% Set up multiple hostname paths via application config + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_paths, [["privateDnsName"], ["privateIpAddress"]]} + ]} + ]), + + %% Test that multiple hostnames are extracted + Expected = ["ip-10-0-16-29.eu-west-1.compute.internal", "10.0.16.29", + "ip-10-0-16-31.eu-west-1.compute.internal", "10.0.16.31"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +single_path_fallback(_Config) -> + %% Test fallback to single path when no multiple paths configured + %% This should use the default single path behavior + Expected = ["ip-10-0-16-29.eu-west-1.compute.internal", + "ip-10-0-16-31.eu-west-1.compute.internal"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +use_private_ip_override_with_multiple_paths(_Config) -> + %% Test that use_private_ip still works with multiple paths + os:putenv("AWS_USE_PRIVATE_IP", "true"), + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_paths, [["privateDnsName"], ["privateIpAddress"]]}, + {aws_use_private_ip, true} + ]} + ]), + + %% Should get private IPs for privateDnsName path (overridden), plus privateIpAddress path + %% After deduplication, should have unique IPs only + Expected = ["10.0.16.29", "10.0.16.31"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +complex_path_extraction(_Config) -> + %% Test extraction from network interface paths + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_paths, [["networkInterfaceSet", 1, "association", "publicDnsName"], + ["privateDnsName"]]} + ]} + ]), + + %% Should extract from both paths + Expected = ["ec2-203-0-113-11.eu-west-1.compute.amazonaws.com", + "ip-10-0-16-29.eu-west-1.compute.internal", + "ec2-203-0-113-21.eu-west-1.compute.amazonaws.com", + "ip-10-0-16-31.eu-west-1.compute.internal"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +hostname_paths_takes_precedence_over_hostname_path(_Config) -> + %% Test that aws_hostname_paths takes precedence over aws_hostname_path + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_path, ["privateIpAddress"]}, % This should be ignored + {aws_hostname_paths, [["privateDnsName"]]} % This should be used + ]} + ]), + + %% Should use hostname_paths (privateDnsName), not hostname_path (privateIpAddress) + Expected = ["ip-10-0-16-29.eu-west-1.compute.internal", + "ip-10-0-16-31.eu-west-1.compute.internal"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +empty_hostname_paths_falls_back_to_hostname_path(_Config) -> + %% Test that empty aws_hostname_paths falls back to aws_hostname_path + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_path, ["privateIpAddress"]}, % This should be used + {aws_hostname_paths, []} % Empty, should fall back + ]} + ]), + + %% Should fall back to hostname_path (privateIpAddress) + Expected = ["10.0.16.29", "10.0.16.31"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +use_private_ip_with_both_hostname_path_and_hostname_paths(_Config) -> + %% Test use_private_ip override when both configurations exist + os:putenv("AWS_USE_PRIVATE_IP", "true"), + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_path, ["privateDnsName"]}, % Should be ignored + {aws_hostname_paths, [["privateDnsName"], ["privateIpAddress"]]}, % Should be used + {aws_use_private_ip, true} + ]} + ]), + + %% Should use hostname_paths with use_private_ip override applied + %% privateDnsName -> privateIpAddress, privateIpAddress stays the same + %% After deduplication: only unique IPs + Expected = ["10.0.16.29", "10.0.16.31"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +use_private_ip_with_fallback_to_hostname_path(_Config) -> + %% Test use_private_ip override when falling back to single hostname_path + os:putenv("AWS_USE_PRIVATE_IP", "true"), + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_path, ["privateDnsName"]}, % Should be used with override + {aws_hostname_paths, []}, % Empty, falls back + {aws_use_private_ip, true} + ]} + ]), + + %% Should fall back to hostname_path with use_private_ip override + %% privateDnsName -> privateIpAddress + Expected = ["10.0.16.29", "10.0.16.31"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set( + reservation_set(), []), + ?assertMatch(Expected, Result). + +out_of_bounds_index_should_not_crash(_Config) -> + os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,3,privateIpAddress"), + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(reservation_set(), []), + ?assertMatch([], Result). + +nested_out_of_bounds_should_not_crash(_Config) -> + os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,2,privateIpAddressesSet,5,privateIpAddress"), + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(reservation_set(), []), + ?assertMatch([], Result). + +zero_index_should_not_crash(_Config) -> + os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,0,privateIpAddress"), + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(reservation_set(), []), + ?assertMatch([], Result). + +multiple_paths_with_graceful_degradation(_Config) -> + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_paths, [ + ["privateDnsName"], % Valid + ["networkInterfaceSet", 10, "privateIpAddress"], % Out of bounds + ["privateIpAddress"] % Valid + ]} + ]} + ]), + Expected = ["ip-10-0-16-29.eu-west-1.compute.internal", "10.0.16.29", + "ip-10-0-16-31.eu-west-1.compute.internal", "10.0.16.31"], + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(reservation_set(), []), + ?assertMatch(Expected, Result). + +empty_network_interface_set_should_not_crash(_Config) -> + EmptyNetworkReservationSet = [ + {"item", [{"reservationId","r-test"}, + {"instancesSet", + [{"item", + [{"instanceId","i-test"}, + {"privateDnsName", "test-host.compute.internal"}, + {"networkInterfaceSet", []}, + {"privateIpAddress","10.0.1.1"}]}]}]} + ], + os:putenv("AWS_HOSTNAME_PATH", "networkInterfaceSet,1,privateIpAddress"), + Result = rabbit_peer_discovery_aws:get_hostname_name_from_reservation_set(EmptyNetworkReservationSet, []), + ?assertMatch([], Result). + +hostname_paths_helper_functions(_Config) -> + %% Test extract_unique_hostnames function + Paths = [["privateDnsName"], ["privateIpAddress"]], + %% Extract items correctly from reservation set structure - reservation_set() returns a list + ReservationSet = reservation_set(), + [{"item", FirstReservation}] = lists:sublist(ReservationSet, 1), + InstancesSet = proplists:get_value("instancesSet", FirstReservation), + Items = [Item || {"item", Item} <- InstancesSet], + + Hostnames = rabbit_peer_discovery_aws:extract_unique_hostnames(Paths, Items), + ?assertEqual(2, length(Hostnames)), % Should extract 2 unique hostnames for the first instance + + %% Test with duplicates - should be automatically removed + DuplicatePaths = [["privateDnsName"], ["privateDnsName"], ["privateIpAddress"]], + UniqueHostnames = rabbit_peer_discovery_aws:extract_unique_hostnames(DuplicatePaths, Items), + ?assertEqual(2, length(UniqueHostnames)), % Should still be 2 unique hostnames + + %% Test empty path handling - should return empty string gracefully + EmptyPath = [], + EmptyResult = rabbit_peer_discovery_aws:get_hostname(EmptyPath, [{"test", "value"}]), + ?assertMatch("", EmptyResult), + + %% Test invalid path handling - should return empty string gracefully + InvalidPath = [invalid_atom], + InvalidResult = rabbit_peer_discovery_aws:get_hostname(InvalidPath, [{"test", "value"}]), + ?assertMatch("", InvalidResult), + + %% Test get_hostname_paths function behavior + %% Test default behavior (no configuration) + DefaultPaths = rabbit_peer_discovery_aws:get_hostname_paths(), + ?assertMatch([["privateDnsName"]], DefaultPaths), + + %% Test with multiple paths configured + application:set_env(rabbit, cluster_formation, [ + {peer_discovery_aws, [ + {aws_hostname_paths, [["privateDnsName"], ["privateIpAddress"]]} + ]} + ]), + MultiplePaths = rabbit_peer_discovery_aws:get_hostname_paths(), + ?assertMatch([["privateDnsName"], ["privateIpAddress"]], MultiplePaths). + +%%% +%%% Implementation +%%% + +reset() -> + ok = application:unset_env(rabbit, cluster_formation), + true = os:unsetenv("AWS_HOSTNAME_PATH"), + true = os:unsetenv("AWS_HOSTNAME_PATHS"), + true = os:unsetenv("AWS_USE_PRIVATE_IP"), + ok. + +reservation_set() -> + [{"item", [{"reservationId","r-006cfdbf8d04c5f01"}, + {"ownerId","248536293561"}, + {"groupSet",[]}, + {"instancesSet", + [{"item", + [{"instanceId","i-0c6d048641f09cad2"}, + {"imageId","ami-ef4c7989"}, + {"instanceState", + [{"code","16"},{"name","running"}]}, + {"privateDnsName", + "ip-10-0-16-29.eu-west-1.compute.internal"}, + {"dnsName",[]}, + {"instanceType","c4.large"}, + {"launchTime","2017-04-07T12:05:10"}, + {"subnetId","subnet-61ff660"}, + {"vpcId","vpc-4fe1562b"}, + {"networkInterfaceSet", [ + {"item", + [{"attachment", [{"deviceIndex", "1"}]}, + {"association", + [{"publicIp","203.0.113.12"}, + {"publicDnsName", + "ec2-203-0-113-12.eu-west-1.compute.amazonaws.com"}, + {"ipOwnerId","amazon"}]}, + {"privateIpAddressesSet", [ + {"item", [ + {"privateIpAddress", "10.0.15.101"}, + {"privateDnsName", "ip-10-0-15-101.eu-west-1.compute.internal"}, + {"primary", "false"} + ]}, + {"item", [ + {"privateIpAddress", "10.0.15.100"}, + {"privateDnsName", "ip-10-0-15-100.eu-west-1.compute.internal"}, + {"primary", "true"} + ]} + ]}]}, + {"item", + [{"attachment", [{"deviceIndex", "0"}]}, + {"association", + [{"publicIp","203.0.113.11"}, + {"publicDnsName", + "ec2-203-0-113-11.eu-west-1.compute.amazonaws.com"}, + {"ipOwnerId","amazon"}]}]}]}, + {"privateIpAddress","10.0.16.29"}]}]}]}, + {"item", [{"reservationId","r-006cfdbf8d04c5f01"}, + {"ownerId","248536293561"}, + {"groupSet",[]}, + {"instancesSet", + [{"item", + [{"instanceId","i-1c6d048641f09cad2"}, + {"imageId","ami-af4c7989"}, + {"instanceState", + [{"code","16"},{"name","running"}]}, + {"privateDnsName", + "ip-10-0-16-31.eu-west-1.compute.internal"}, + {"dnsName",[]}, + {"instanceType","c4.large"}, + {"launchTime","2017-04-07T12:05:10"}, + {"subnetId","subnet-61ff660"}, + {"vpcId","vpc-4fe1562b"}, + {"networkInterfaceSet", [ + {"item", + [{"attachment", [{"deviceIndex", "0"}]}, + {"association", + [{"publicIp","203.0.113.21"}, + {"publicDnsName", + "ec2-203-0-113-21.eu-west-1.compute.amazonaws.com"}, + {"ipOwnerId","amazon"}]}]}, + {"item", + [{"attachment", [{"deviceIndex", "1"}]}, + {"association", + [{"publicIp","203.0.113.22"}, + {"publicDnsName", + "ec2-203-0-113-22.eu-west-1.compute.amazonaws.com"}, + {"ipOwnerId","amazon"}]}, + {"privateIpAddressesSet", [ + {"item", [ + {"privateIpAddress", "10.0.16.31"}, + {"privateDnsName", "ip-10-0-16-31.eu-west-1.compute.internal"}, + {"primary", "true"} + ]} + ]}]}]}, + {"privateIpAddress","10.0.16.31"}]}]}]}]. From 2a3274a96671e7f7ae8340f952a5333e03eb2621 Mon Sep 17 00:00:00 2001 From: Luke Bakken Date: Wed, 22 Oct 2025 13:35:59 -0700 Subject: [PATCH 2/2] Update deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema Co-authored-by: Michael Davis --- .../schema/rabbitmq_peer_discovery_aws.schema | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema b/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema index 1f46ccfde1be..7c6b91a95625 100644 --- a/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema +++ b/deps/rabbitmq_peer_discovery_aws/priv/schema/rabbitmq_peer_discovery_aws.schema @@ -117,23 +117,23 @@ end}. {translation, "rabbit.cluster_formation.peer_discovery_aws.aws_hostname_paths", fun(Conf) -> - Settings = cuttlefish_variable:filter_by_prefix("cluster_formation.aws.hostname_path", Conf), - %% Helper function for number validation - IsNumberString = fun(Str) -> - case string:to_integer(Str) of - {_, ""} -> true; - _ -> false - end - end, - %% Extract and sort numbered paths (hostname_path.1, hostname_path.2, etc.) - NumberedPaths = [{list_to_integer(N), V} || - {["cluster_formation", "aws", "hostname_path", N], V} <- Settings, - IsNumberString(N)], - case NumberedPaths of - [] -> cuttlefish:unset(); - _ -> - SortedPaths = lists:sort(NumberedPaths), - [rabbit_peer_discovery_util:as_list(V) || {_, V} <- SortedPaths] + case cuttlefish_variable:filter_by_prefix("cluster_formation.aws.hostname_path", Conf) of + [] -> + cuttlefish:unset(); + L -> + %% Extract and sort numbered paths (hostname_path.1, hostname_path.2, etc.) + L1 = lists:map( + fun ({["cluster_formation", "aws", "hostname_path", N], V}) -> + case string:to_integer(N) of + {I, ""} -> + {I, V}; + _ -> + cuttlefish:invalid(io_lib:format("Cannot convert ~p to an integer", [N])) + end; + (Other) -> + cuttlefish:invalid(io_lib:format("~p is invalid", [Other])) + end, L), + [rabbit_peer_discovery_util:as_list(V) || {_, V} <- lists:sort(L1)] end end}.