add rpc_public_addr_subnet config option #817

Open
flokli wants to merge 1 commit from flokli/garage:rpc_public_addr_subnet into main
8 changed files with 59 additions and 9 deletions

1
Cargo.lock generated
View file

@ -1527,6 +1527,7 @@ dependencies = [
"garage_util",
"gethostname",
"hex",
"ipnet",
"itertools 0.12.1",
"k8s-openapi",
"kube",

View file

@ -34,7 +34,7 @@ args@{
ignoreLockHash,
}:
let
nixifiedLockHash = "1ccd5eb25a83962821e0e9da4ce6df31717b2b97a5b3a0c80c9e0e0759710143";
nixifiedLockHash = "fc41fb639a69d62c8c0fb3f9c227162162ebc8142c6fa5cd0599dc381dcd9ebb";
workspaceSrc = if args.workspaceSrc == null then ./. else args.workspaceSrc;
currentLockHash = builtins.hashFile "sha256" (workspaceSrc + /Cargo.lock);
lockHashIgnored = if ignoreLockHash
@ -2219,6 +2219,7 @@ in
garage_util = (rustPackages."unknown".garage_util."1.0.0" { inherit profileName; }).out;
gethostname = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".gethostname."0.4.3" { inherit profileName; }).out;
hex = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".hex."0.4.3" { inherit profileName; }).out;
ipnet = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".ipnet."2.9.0" { inherit profileName; }).out;
itertools = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".itertools."0.12.1" { inherit profileName; }).out;
${ if rootFeatures' ? "garage/kubernetes-discovery" || rootFeatures' ? "garage_rpc/k8s-openapi" || rootFeatures' ? "garage_rpc/kubernetes-discovery" then "k8s_openapi" else null } = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".k8s-openapi."0.21.0" { inherit profileName; }).out;
${ if rootFeatures' ? "garage/kubernetes-discovery" || rootFeatures' ? "garage_rpc/kube" || rootFeatures' ? "garage_rpc/kubernetes-discovery" then "kube" else null } = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".kube."0.88.1" { inherit profileName; }).out;
@ -3016,8 +3017,8 @@ in
registry = "registry+https://github.com/rust-lang/crates.io-index";
src = fetchCratesIo { inherit name version; sha256 = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3"; };
features = builtins.concatLists [
(lib.optional (rootFeatures' ? "garage/consul-discovery" || rootFeatures' ? "garage_rpc/consul-discovery" || rootFeatures' ? "garage_rpc/reqwest") "default")
(lib.optional (rootFeatures' ? "garage/consul-discovery" || rootFeatures' ? "garage_rpc/consul-discovery" || rootFeatures' ? "garage_rpc/reqwest") "std")
[ "default" ]
[ "std" ]
];
});

View file

@ -55,6 +55,7 @@ hexdump = "0.1"
hmac = "0.12"
idna = "0.5"
itertools = "0.12"
ipnet = "2.9.0"

This was already in the list of dependencies (through reqwest), but we now explicitly depend on it, too.

This was already in the list of dependencies (through `reqwest`), but we now explicitly depend on it, too.
lazy_static = "1.4"
md-5 = "0.10"
mktemp = "0.5"

View file

@ -152,6 +152,8 @@ Check the following for your configuration files:
- Make sure `rpc_public_addr` contains the public IP address of the node you are configuring.
This parameter is optional but recommended: if your nodes have trouble communicating with
one another, consider adding it.
Alternatively, you can also set `rpc_public_addr_subnet`, which can filter
the addresses announced to other peers to a specific subnet.
- Make sure `rpc_secret` is the same value on all nodes. It should be a 32-bytes hex-encoded secret key.
You can generate such a key with `openssl rand -hex 32`.

View file

@ -31,6 +31,9 @@ rpc_secret = "4425f5c26c5e11581d3223904324dcb5b5d5dfb14e5e7f35e38c595424f5f1e6"
rpc_bind_addr = "[::]:3901"
rpc_bind_outgoing = false
rpc_public_addr = "[fc00:1::1]:3901"
# or set rpc_public_adr_subnet to filter down autodiscovery to a subnet:
# rpc_public_addr_subnet = "2001:0db8:f00:b00:/64"
allow_world_readable_secrets = false
@ -543,6 +546,14 @@ RPC calls. **This parameter is optional but recommended.** In case you have
a NAT that binds the RPC port to a port that is different on your public IP,
Outdated
Review

New options should also be added in the example config file at the beginning of the page (that contains an instance of all configuration parameters), as well as in the index/list of everything at the beginning of "available configuration options".

New options should also be added in the example config file at the beginning of the page (that contains an instance of all configuration parameters), as well as in the index/list of everything at the beginning of "available configuration options".

rpc_public_addr_subnet only applies if rpc_public_addr is not set (as it influences autodiscovery), which only happens if that is not set.

I added a comment, and a commented-out example value.

`rpc_public_addr_subnet` only applies if `rpc_public_addr` is not set (as it influences autodiscovery), which only happens if that is not set. I added a comment, and a commented-out example value.
this field might help making it work.
#### `rpc_public_addr_subnet` {#rpc_public_addr_subnet}
Review

A reference should also be added in the list that starts on line 93

A reference should also be added in the list that starts on line 93
In case `rpc_public_addr` is not set, but autodiscovery is used, this allows
filtering the list of automatically discovered IPs to a specific subnet.
For example, if nodes should pick *their* IP inside a specific subnet, but you
don't want to explicitly write the IP down (as it's dynamic, or you want to
share configs across nodes), you can use this option.
#### `bootstrap_peers` {#bootstrap_peers}
A list of peer identifiers on which to contact other Garage peers of this cluster.

View file

@ -24,6 +24,7 @@ bytes.workspace = true
bytesize.workspace = true
gethostname.workspace = true
hex.workspace = true
ipnet.workspace = true
tracing.workspace = true
rand.workspace = true
itertools.workspace = true

View file

@ -844,12 +844,20 @@ impl NodeStatus {
}
}
fn get_default_ip() -> Option<IpAddr> {
/// Obtain the list of currently available IP addresses on all non-loopback
/// interfaces, optionally filtering them to be inside a given IpNet.
fn get_default_ip(filter_ipnet: Option<ipnet::IpNet>) -> Option<IpAddr> {
pnet_datalink::interfaces()
.iter()
.find(|e| e.is_up() && !e.is_loopback() && !e.ips.is_empty())
.and_then(|e| e.ips.first())
.map(|a| a.ip())
.into_iter()
// filter down and loopback interfaces
.filter(|i| i.is_up() && !i.is_loopback())
// get all IPs
.flat_map(|e| e.ips)
// optionally, filter to be inside filter_ipnet
.find(|ipn| {
filter_ipnet.is_some_and(|ipnet| ipnet.contains(&ipn.ip())) || filter_ipnet.is_none()
})
.map(|ipn| ipn.ip())
}
fn get_rpc_public_addr(config: &Config) -> Option<SocketAddr> {
@ -877,7 +885,28 @@ fn get_rpc_public_addr(config: &Config) -> Option<SocketAddr> {
}
}
None => {
let addr = get_default_ip().map(|ip| SocketAddr::new(ip, config.rpc_bind_addr.port()));
// `No rpc_public_addr` specified, try to discover one, optionally filtering by `rpc_public_addr_subnet`.
let filter_subnet: Option<ipnet::IpNet> = config
.rpc_public_addr_subnet
.as_ref()
.and_then(|filter_subnet_str| match filter_subnet_str.parse::<ipnet::IpNet>() {
Ok(filter_subnet) => {
let filter_subnet_trunc = filter_subnet.trunc();
if filter_subnet.trunc() != filter_subnet {
Review

this code calls .trunc() again instead of using the filter_subnet_trunc variable

this code calls `.trunc()` again instead of using the `filter_subnet_trunc` variable
warn!("`rpc_public_addr_subnet` changed after applying netmask, continuing with {}", filter_subnet.trunc());
}
Some(filter_subnet_trunc)
}
Err(e) => {
panic!(
"Cannot parse rpc_public_addr_subnet {} from config file: {}. Bailing out.",
filter_subnet_str, e
);
Outdated
Review

I think we probably want this to be a hard error that makes garage exit immediately, because the only reason for this is a syntax error in the config file which is the admin's fault

I think we probably want this to be a hard error that makes garage exit immediately, because the only reason for this is a syntax error in the config file which is the admin's fault

I made this a panic!. This function doesn't return errors, and threading that through would be a bit more of a diff.
It seems panic! is also commonly used in other places that deal with config parsing.

Long-term, we probably want to move these parsing concerns to much earlier into the CLI code, not that late, but that also feels out of scope for this PR.

I made this a `panic!`. This function doesn't return errors, and threading that through would be a bit more of a diff. It seems `panic!` is also commonly used in other places that deal with config parsing. Long-term, we probably want to move these parsing concerns to much earlier into the CLI code, not that late, but that also feels out of scope for this PR.
}
});
let addr = get_default_ip(filter_subnet)
.map(|ip| SocketAddr::new(ip, config.rpc_bind_addr.port()));
if let Some(a) = addr {
warn!("Using autodetected rpc_public_addr: {}. Consider specifying it explicitly in configuration file if possible.", a);
}

View file

@ -85,6 +85,10 @@ pub struct Config {
/// Public IP address of this node
pub rpc_public_addr: Option<String>,
/// In case `rpc_public_addr` was not set, this can filter
/// the addresses announced to other peers to a specific subnet.
pub rpc_public_addr_subnet: Option<String>,
/// Timeout for Netapp's ping messagess
pub rpc_ping_timeout_msec: Option<u64>,
/// Timeout for Netapp RPC calls