From 088c200b56b0e8d2944ae4210058da3b2654c567 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Mon, 7 Oct 2024 21:25:44 +0200 Subject: [PATCH 1/3] feat(csi-node): allow listening on IPv6 Pod IPs Signed-off-by: Mike Beaumont --- .../csi-driver/src/bin/node/main_.rs | 53 ++++++++++++++----- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/control-plane/csi-driver/src/bin/node/main_.rs b/control-plane/csi-driver/src/bin/node/main_.rs index cb245e768..2cdb7646d 100644 --- a/control-plane/csi-driver/src/bin/node/main_.rs +++ b/control-plane/csi-driver/src/bin/node/main_.rs @@ -29,7 +29,7 @@ use std::{ env, fs, future::Future, io::ErrorKind, - net::SocketAddr, + net::{IpAddr, SocketAddr}, pin::Pin, str::FromStr, sync::Arc, @@ -136,6 +136,24 @@ pub(super) async fn main() -> anyhow::Result<()> { .default_value("[::]") .required(false) ) + .arg( + Arg::new("grpc-ip") + .long("grpc-ip") + .value_parser(clap::value_parser!(IpAddr)) + .value_name("GRPC_IP") + .help("ip address this instance listens on") + .default_value("::") + .required(false) + ) + .arg( + Arg::new("grpc-port") + .long("grpc-port") + .value_parser(clap::value_parser!(u16)) + .value_name("GRPC_PORT") + .help("port this instance listens on") + .default_value(GRPC_PORT.to_string()) + .required(false) + ) .arg( Arg::new("v") .short('v') @@ -330,10 +348,7 @@ pub(super) async fn main() -> anyhow::Result<()> { let registration_enabled = matches.get_flag("enable-registration"); // Parse instance and grpc endpoints from the command line arguments and validate. - let grpc_sock_addr = validate_endpoints( - matches.get_one::("grpc-endpoint").unwrap(), - registration_enabled, - )?; + let grpc_sock_addr = validate_endpoints(&matches, registration_enabled)?; // Start the CSI server, node plugin grpc server and registration loop if registration is // enabled. @@ -427,21 +442,35 @@ async fn check_ana_and_label_node( /// Validate that the grpc endpoint is valid. fn validate_endpoints( - grpc_endpoint: &str, + matches: &clap::ArgMatches, registration_enabled: bool, ) -> anyhow::Result { - let grpc_endpoint = if grpc_endpoint.contains(':') { - grpc_endpoint.to_string() + let grpc_endpoint_src = matches.value_source("grpc-endpoint"); + let grpc_ip = matches.get_one::("grpc-ip"); + let grpc_port = matches.get_one::("grpc-port"); + let grpc_endpoint_socket_addr = if grpc_endpoint_src + .unwrap_or(clap::parser::ValueSource::DefaultValue) + == clap::parser::ValueSource::DefaultValue + { + SocketAddr::new( + *grpc_ip.expect("must be provided if grpc-endpoint is missing"), + *grpc_port.expect("must be provided if grpc-port is missing"), + ) } else { - format!("{grpc_endpoint}:{GRPC_PORT}") + let grpc_endpoint_arg = matches.get_one::("grpc-endpoint").unwrap(); + let grpc_endpoint = if grpc_endpoint_arg.contains(':') { + grpc_endpoint_arg.to_string() + } else { + format!("{grpc_endpoint_arg}:{GRPC_PORT}") + }; + SocketAddr::from_str(&grpc_endpoint)? }; - let grpc_endpoint_url = SocketAddr::from_str(&grpc_endpoint)?; // Should not allow using an unspecified ip if registration is enabled as grpc endpoint gets // sent in registration request. - if registration_enabled && grpc_endpoint_url.ip().is_unspecified() { + if registration_enabled && grpc_endpoint_socket_addr.ip().is_unspecified() { return Err(anyhow::format_err!( "gRPC endpoint: `[::]`/`0.0.0.0` is not allowed if registration is enabled" )); } - Ok(grpc_endpoint_url) + Ok(grpc_endpoint_socket_addr) } From 87ec9a51f16b9e6662c48ef6439e0e2b3e07f18c Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Tue, 8 Oct 2024 21:06:05 +0200 Subject: [PATCH 2/3] feat: add conflicts_with_all Signed-off-by: Mike Beaumont --- control-plane/csi-driver/src/bin/node/main_.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/control-plane/csi-driver/src/bin/node/main_.rs b/control-plane/csi-driver/src/bin/node/main_.rs index 2cdb7646d..34b1ea417 100644 --- a/control-plane/csi-driver/src/bin/node/main_.rs +++ b/control-plane/csi-driver/src/bin/node/main_.rs @@ -132,6 +132,7 @@ pub(super) async fn main() -> anyhow::Result<()> { .short('g') .long("grpc-endpoint") .value_name("ENDPOINT") + .conflicts_with_all(["grpc-ip", "grpc-port"]) .help("ip address where this instance runs, and optionally the gRPC port") .default_value("[::]") .required(false) From 27109e4c49d40bae3f62cd83689394da0499c036 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Wed, 9 Oct 2024 20:35:10 +0200 Subject: [PATCH 3/3] refactor: improve grpc endpoint validation Signed-off-by: Mike Beaumont --- .../csi-driver/src/bin/node/main_.rs | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/control-plane/csi-driver/src/bin/node/main_.rs b/control-plane/csi-driver/src/bin/node/main_.rs index 34b1ea417..7898c8d7f 100644 --- a/control-plane/csi-driver/src/bin/node/main_.rs +++ b/control-plane/csi-driver/src/bin/node/main_.rs @@ -134,7 +134,6 @@ pub(super) async fn main() -> anyhow::Result<()> { .value_name("ENDPOINT") .conflicts_with_all(["grpc-ip", "grpc-port"]) .help("ip address where this instance runs, and optionally the gRPC port") - .default_value("[::]") .required(false) ) .arg( @@ -446,25 +445,22 @@ fn validate_endpoints( matches: &clap::ArgMatches, registration_enabled: bool, ) -> anyhow::Result { - let grpc_endpoint_src = matches.value_source("grpc-endpoint"); + let grpc_endpoint = matches.get_one::("grpc-endpoint"); let grpc_ip = matches.get_one::("grpc-ip"); let grpc_port = matches.get_one::("grpc-port"); - let grpc_endpoint_socket_addr = if grpc_endpoint_src - .unwrap_or(clap::parser::ValueSource::DefaultValue) - == clap::parser::ValueSource::DefaultValue - { - SocketAddr::new( - *grpc_ip.expect("must be provided if grpc-endpoint is missing"), - *grpc_port.expect("must be provided if grpc-port is missing"), - ) - } else { - let grpc_endpoint_arg = matches.get_one::("grpc-endpoint").unwrap(); - let grpc_endpoint = if grpc_endpoint_arg.contains(':') { - grpc_endpoint_arg.to_string() - } else { - format!("{grpc_endpoint_arg}:{GRPC_PORT}") - }; - SocketAddr::from_str(&grpc_endpoint)? + let grpc_endpoint_socket_addr = match grpc_endpoint { + None => SocketAddr::new( + *grpc_ip.expect("grpc-ip must be provided if grpc-endpoint is missing"), + *grpc_port.expect("grpc-port must be provided if grpc-endpoint is missing"), + ), + Some(grpc_endpoint) => { + let grpc_endpoint = if grpc_endpoint.contains(':') { + grpc_endpoint.to_string() + } else { + format!("{grpc_endpoint}:{GRPC_PORT}") + }; + SocketAddr::from_str(&grpc_endpoint)? + } }; // Should not allow using an unspecified ip if registration is enabled as grpc endpoint gets // sent in registration request.