hubcio commented on code in PR #4218:
URL: https://github.com/apache/iggy/pull/4218#discussion_r4045535131
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
- return Ok(());
+ return Ok(None);
};
let bind = parse_bind_address(listener.key, listener.address)?;
- if !bind.ip().to_canonical().is_unspecified() {
- return Ok(());
+ let ip = bind.ip().to_canonical();
+ if ip.is_unspecified() {
+ if self.node.advertised_address.is_none() {
+ eprintln!(
+ "{COMPONENT} - {} binds the wildcard {bind}, which says
which interfaces this node \
+ accepts on rather than where a client reaches it, so
cluster metadata would carry no \
+ address for this node. Set node.advertised_address to the
address clients dial, or \
+ bind a concrete address.",
+ listener.key
+ );
+ return Err(ConfigurationError::InvalidConfigurationValue);
+ }
+ return Ok(None);
}
- eprintln!(
- "{COMPONENT} - {} binds the wildcard {bind}, which says which
interfaces this node \
- accepts on rather than where a client reaches it, so cluster
metadata would carry no \
- address for this node. Set node.advertised_address to the address
clients dial, or \
- bind a concrete address.",
- listener.key
- );
- Err(ConfigurationError::InvalidConfigurationValue)
+ if ip.is_loopback() && is_container {
Review Comment:
warning: under `docker run --network host` the container shares the host
network namespace, so same-host clients do reach this bind and the message is
wrong. name the host-network case.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
Review Comment:
nit: `is_container()` runs on every boot, even when the first check returns.
move the probe into the loopback branch.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
- return Ok(());
+ return Ok(None);
};
let bind = parse_bind_address(listener.key, listener.address)?;
Review Comment:
nit: this drops the `advertised_address` shortcut, so a malformed listener
address now fails validation instead of boot. both paths refuse, so a commit
body note is enough.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -534,6 +623,116 @@ mod tests {
assert!(config.validate().is_ok());
}
+ #[test]
+ fn given_loopback_bind_in_container_when_validating_should_warn_and_pass()
{
+ let config = config_with_override(
+ "[tcp]\naddress = \"127.0.0.1:8090\"\n[cluster]\nenabled =
false\n",
+ );
+ let warning = config
+ .validate_client_facing_address_in_env(true)
+ .expect("validation should pass");
+ assert!(
+ warning.is_some(),
+ "loopback inside container must produce a warning"
+ );
+ let message = warning.unwrap();
+ assert!(message.contains("IGGY_TCP_ADDRESS=0.0.0.0:8090"));
+ assert!(message.contains("together with
IGGY_NODE_ADVERTISED_ADDRESS"));
+ }
+
+ #[test]
+ fn
given_loopback_bind_outside_container_when_validating_should_pass_without_warning()
{
+ let config = config_with_override(
+ "[tcp]\naddress = \"127.0.0.1:8090\"\n[cluster]\nenabled =
false\n",
+ );
+ let warning = config
+ .validate_client_facing_address_in_env(false)
+ .expect("validation should pass");
+ assert!(
+ warning.is_none(),
+ "loopback outside container must not produce a warning"
+ );
+ }
+
+ #[test]
+ fn
given_loopback_bind_in_container_with_advertised_address_when_validating_should_warn_and_pass()
+ {
+ let config = config_with_override(
+ "[tcp]\naddress = \"127.0.0.1:8090\"\n[cluster]\nenabled = false\n\
+ [node]\nadvertised_address = \"broker-1.example.com\"\n",
+ );
+ let warning = config
+ .validate_client_facing_address_in_env(true)
+ .expect("validation should pass");
+ assert!(
+ warning.is_some(),
+ "loopback inside container must produce a warning"
+ );
+ let message = warning.unwrap();
+ assert!(message.contains("IGGY_TCP_ADDRESS=0.0.0.0:8090"));
+ assert!(!message.contains("together with
IGGY_NODE_ADVERTISED_ADDRESS"));
+ }
+
+ #[test]
+ fn given_dockerenv_file_when_checking_container_should_return_true() {
+ let temp_dir = std::env::temp_dir();
+ let marker = temp_dir.join(format!("test_dockerenv_{}",
std::process::id()));
+ std::fs::write(&marker, "").unwrap();
+ let non_existent = temp_dir.join("non_existent_indicator");
+ let result =
+ is_container_indicators(&marker, &non_existent, None, None,
"/non/existent/cgroup");
+ let _ = std::fs::remove_file(&marker);
+ assert!(result);
+ }
+
+ #[test]
+ fn given_containerenv_file_when_checking_container_should_return_true() {
+ let temp_dir = std::env::temp_dir();
+ let marker = temp_dir.join(format!("test_containerenv_{}",
std::process::id()));
+ std::fs::write(&marker, "").unwrap();
+ let non_existent = temp_dir.join("non_existent_indicator");
+ let result =
+ is_container_indicators(&non_existent, &marker, None, None,
"/non/existent/cgroup");
+ let _ = std::fs::remove_file(&marker);
+ assert!(result);
+ }
Review Comment:
simplification: both tests write marker files into the shared temp dir and
ignore the cleanup error. use one temp dir for the pair.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
Review Comment:
warning: this warns for the derived listener only, so once an operator sets
`IGGY_TCP_ADDRESS=0.0.0.0:8090` the other three loopback binds go silent for
good. loop over `client_listeners()` and warn per enabled listener.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
- return Ok(());
+ return Ok(None);
};
let bind = parse_bind_address(listener.key, listener.address)?;
- if !bind.ip().to_canonical().is_unspecified() {
- return Ok(());
+ let ip = bind.ip().to_canonical();
+ if ip.is_unspecified() {
+ if self.node.advertised_address.is_none() {
+ eprintln!(
+ "{COMPONENT} - {} binds the wildcard {bind}, which says
which interfaces this node \
+ accepts on rather than where a client reaches it, so
cluster metadata would carry no \
+ address for this node. Set node.advertised_address to the
address clients dial, or \
+ bind a concrete address.",
+ listener.key
+ );
+ return Err(ConfigurationError::InvalidConfigurationValue);
+ }
+ return Ok(None);
}
- eprintln!(
- "{COMPONENT} - {} binds the wildcard {bind}, which says which
interfaces this node \
- accepts on rather than where a client reaches it, so cluster
metadata would carry no \
- address for this node. Set node.advertised_address to the address
clients dial, or \
- bind a concrete address.",
- listener.key
- );
- Err(ConfigurationError::InvalidConfigurationValue)
+ if ip.is_loopback() && is_container {
+ let env_var = format!("IGGY_{}", listener.key.replace('.',
"_").to_uppercase());
Review Comment:
warning: the variable name is rebuilt from `listener.key`, so a renamed key
prints a name that refuses boot. use `ConfigEnvMappings::find_by_config_path()`.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
- return Ok(());
+ return Ok(None);
};
let bind = parse_bind_address(listener.key, listener.address)?;
- if !bind.ip().to_canonical().is_unspecified() {
- return Ok(());
+ let ip = bind.ip().to_canonical();
+ if ip.is_unspecified() {
+ if self.node.advertised_address.is_none() {
+ eprintln!(
+ "{COMPONENT} - {} binds the wildcard {bind}, which says
which interfaces this node \
+ accepts on rather than where a client reaches it, so
cluster metadata would carry no \
+ address for this node. Set node.advertised_address to the
address clients dial, or \
+ bind a concrete address.",
+ listener.key
+ );
+ return Err(ConfigurationError::InvalidConfigurationValue);
+ }
+ return Ok(None);
}
- eprintln!(
- "{COMPONENT} - {} binds the wildcard {bind}, which says which
interfaces this node \
- accepts on rather than where a client reaches it, so cluster
metadata would carry no \
- address for this node. Set node.advertised_address to the address
clients dial, or \
- bind a concrete address.",
- listener.key
- );
- Err(ConfigurationError::InvalidConfigurationValue)
+ if ip.is_loopback() && is_container {
+ let env_var = format!("IGGY_{}", listener.key.replace('.',
"_").to_uppercase());
+ let port = bind.port();
+ let msg = if self.node.advertised_address.is_none() {
+ format!(
+ "{COMPONENT} - {} binds the loopback address {bind} inside
a container; the \
+ server will not be reachable from outside the container.
Set {env_var}=0.0.0.0:{port} \
+ together with IGGY_NODE_ADVERTISED_ADDRESS, or bind a
concrete address.",
+ listener.key
+ )
+ } else {
+ format!(
+ "{COMPONENT} - {} binds the loopback address {bind} inside
a container; the \
+ server will not be reachable from outside the container.
Set {env_var}=0.0.0.0:{port} \
+ or bind a concrete address.",
+ listener.key
+ )
+ };
+ warn!("{msg}");
+ return Ok(Some(msg));
+ }
+
+ Ok(None)
+ }
+}
+
+/// Returns true when the process is executing inside a container.
+fn is_container() -> bool {
+ is_container_indicators(
+ Path::new("/.dockerenv"),
+ Path::new("/run/.containerenv"),
+ std::env::var_os("container").as_deref(),
+ std::env::var_os("KUBERNETES_SERVICE_HOST").as_deref(),
+ "/proc/self/cgroup",
+ )
+}
+
+fn is_container_indicators(
+ dockerenv_path: &Path,
+ containerenv_path: &Path,
+ container_env: Option<&OsStr>,
+ k8s_env: Option<&OsStr>,
+ cgroup_path: &str,
+) -> bool {
+ if dockerenv_path.exists() || containerenv_path.exists() {
+ return true;
}
+
+ if container_env.is_some() || k8s_env.is_some() {
+ return true;
+ }
+
+ #[cfg(target_os = "linux")]
+ {
+ if let Ok(cgroup) = std::fs::read_to_string(cgroup_path)
Review Comment:
nit: with a private cgroup namespace cgroup v2 reports `0::/`, so no marker
matches and the container goes undetected. compare the namespace inode against
PID 1, or drop the branch.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
- return Ok(());
+ return Ok(None);
};
let bind = parse_bind_address(listener.key, listener.address)?;
- if !bind.ip().to_canonical().is_unspecified() {
- return Ok(());
+ let ip = bind.ip().to_canonical();
+ if ip.is_unspecified() {
+ if self.node.advertised_address.is_none() {
+ eprintln!(
+ "{COMPONENT} - {} binds the wildcard {bind}, which says
which interfaces this node \
+ accepts on rather than where a client reaches it, so
cluster metadata would carry no \
+ address for this node. Set node.advertised_address to the
address clients dial, or \
+ bind a concrete address.",
+ listener.key
+ );
+ return Err(ConfigurationError::InvalidConfigurationValue);
+ }
+ return Ok(None);
}
- eprintln!(
- "{COMPONENT} - {} binds the wildcard {bind}, which says which
interfaces this node \
- accepts on rather than where a client reaches it, so cluster
metadata would carry no \
- address for this node. Set node.advertised_address to the address
clients dial, or \
- bind a concrete address.",
- listener.key
- );
- Err(ConfigurationError::InvalidConfigurationValue)
+ if ip.is_loopback() && is_container {
+ let env_var = format!("IGGY_{}", listener.key.replace('.',
"_").to_uppercase());
+ let port = bind.port();
+ let msg = if self.node.advertised_address.is_none() {
+ format!(
+ "{COMPONENT} - {} binds the loopback address {bind} inside
a container; the \
+ server will not be reachable from outside the container.
Set {env_var}=0.0.0.0:{port} \
+ together with IGGY_NODE_ADVERTISED_ADDRESS, or bind a
concrete address.",
+ listener.key
+ )
+ } else {
+ format!(
+ "{COMPONENT} - {} binds the loopback address {bind} inside
a container; the \
+ server will not be reachable from outside the container.
Set {env_var}=0.0.0.0:{port} \
+ or bind a concrete address.",
+ listener.key
+ )
+ };
+ warn!("{msg}");
+ return Ok(Some(msg));
+ }
+
+ Ok(None)
+ }
+}
+
+/// Returns true when the process is executing inside a container.
+fn is_container() -> bool {
+ is_container_indicators(
+ Path::new("/.dockerenv"),
+ Path::new("/run/.containerenv"),
+ std::env::var_os("container").as_deref(),
+ std::env::var_os("KUBERNETES_SERVICE_HOST").as_deref(),
+ "/proc/self/cgroup",
+ )
+}
+
+fn is_container_indicators(
+ dockerenv_path: &Path,
+ containerenv_path: &Path,
+ container_env: Option<&OsStr>,
+ k8s_env: Option<&OsStr>,
+ cgroup_path: &str,
+) -> bool {
+ if dockerenv_path.exists() || containerenv_path.exists() {
+ return true;
}
+
+ if container_env.is_some() || k8s_env.is_some() {
+ return true;
+ }
+
+ #[cfg(target_os = "linux")]
+ {
+ if let Ok(cgroup) = std::fs::read_to_string(cgroup_path)
+ && cgroup.lines().any(|line| {
+ line.contains("/docker/")
+ || line.contains("/docker-")
+ || line.contains("/libpod-")
+ || line.contains("/podman/")
+ || line.contains("/kubepods/")
+ || line.contains("/kubepods-")
+ || line.contains("/containerd/")
+ || line.contains("/lxc/")
+ })
Review Comment:
simplification: eight marker strings sit inline in one boolean chain. pull
them into a `const` slice and use `.iter().any()`.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -534,6 +623,116 @@ mod tests {
assert!(config.validate().is_ok());
}
+ #[test]
+ fn given_loopback_bind_in_container_when_validating_should_warn_and_pass()
{
+ let config = config_with_override(
+ "[tcp]\naddress = \"127.0.0.1:8090\"\n[cluster]\nenabled =
false\n",
+ );
+ let warning = config
+ .validate_client_facing_address_in_env(true)
+ .expect("validation should pass");
+ assert!(
+ warning.is_some(),
+ "loopback inside container must produce a warning"
+ );
+ let message = warning.unwrap();
+ assert!(message.contains("IGGY_TCP_ADDRESS=0.0.0.0:8090"));
+ assert!(message.contains("together with
IGGY_NODE_ADVERTISED_ADDRESS"));
+ }
+
+ #[test]
+ fn
given_loopback_bind_outside_container_when_validating_should_pass_without_warning()
{
+ let config = config_with_override(
+ "[tcp]\naddress = \"127.0.0.1:8090\"\n[cluster]\nenabled =
false\n",
+ );
+ let warning = config
+ .validate_client_facing_address_in_env(false)
+ .expect("validation should pass");
+ assert!(
+ warning.is_none(),
+ "loopback outside container must not produce a warning"
+ );
+ }
+
+ #[test]
+ fn
given_loopback_bind_in_container_with_advertised_address_when_validating_should_warn_and_pass()
+ {
+ let config = config_with_override(
+ "[tcp]\naddress = \"127.0.0.1:8090\"\n[cluster]\nenabled = false\n\
+ [node]\nadvertised_address = \"broker-1.example.com\"\n",
+ );
+ let warning = config
+ .validate_client_facing_address_in_env(true)
+ .expect("validation should pass");
+ assert!(
+ warning.is_some(),
+ "loopback inside container must produce a warning"
+ );
+ let message = warning.unwrap();
+ assert!(message.contains("IGGY_TCP_ADDRESS=0.0.0.0:8090"));
+ assert!(!message.contains("together with
IGGY_NODE_ADVERTISED_ADDRESS"));
+ }
+
+ #[test]
+ fn given_dockerenv_file_when_checking_container_should_return_true() {
Review Comment:
nit: every test injects its own path or value, so `is_container()` and the
cgroup branch have no coverage and a mistyped sentinel passes. add one test
that writes a docker cgroup line to a temp file.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
- return Ok(());
+ return Ok(None);
};
let bind = parse_bind_address(listener.key, listener.address)?;
- if !bind.ip().to_canonical().is_unspecified() {
- return Ok(());
+ let ip = bind.ip().to_canonical();
+ if ip.is_unspecified() {
+ if self.node.advertised_address.is_none() {
+ eprintln!(
+ "{COMPONENT} - {} binds the wildcard {bind}, which says
which interfaces this node \
+ accepts on rather than where a client reaches it, so
cluster metadata would carry no \
+ address for this node. Set node.advertised_address to the
address clients dial, or \
+ bind a concrete address.",
+ listener.key
+ );
+ return Err(ConfigurationError::InvalidConfigurationValue);
+ }
+ return Ok(None);
}
- eprintln!(
- "{COMPONENT} - {} binds the wildcard {bind}, which says which
interfaces this node \
- accepts on rather than where a client reaches it, so cluster
metadata would carry no \
- address for this node. Set node.advertised_address to the address
clients dial, or \
- bind a concrete address.",
- listener.key
- );
- Err(ConfigurationError::InvalidConfigurationValue)
+ if ip.is_loopback() && is_container {
+ let env_var = format!("IGGY_{}", listener.key.replace('.',
"_").to_uppercase());
+ let port = bind.port();
+ let msg = if self.node.advertised_address.is_none() {
+ format!(
+ "{COMPONENT} - {} binds the loopback address {bind} inside
a container; the \
+ server will not be reachable from outside the container.
Set {env_var}=0.0.0.0:{port} \
+ together with IGGY_NODE_ADVERTISED_ADDRESS, or bind a
concrete address.",
+ listener.key
+ )
+ } else {
+ format!(
+ "{COMPONENT} - {} binds the loopback address {bind} inside
a container; the \
+ server will not be reachable from outside the container.
Set {env_var}=0.0.0.0:{port} \
+ or bind a concrete address.",
+ listener.key
+ )
+ };
+ warn!("{msg}");
+ return Ok(Some(msg));
+ }
+
+ Ok(None)
+ }
+}
+
+/// Returns true when the process is executing inside a container.
+fn is_container() -> bool {
+ is_container_indicators(
+ Path::new("/.dockerenv"),
+ Path::new("/run/.containerenv"),
+ std::env::var_os("container").as_deref(),
+ std::env::var_os("KUBERNETES_SERVICE_HOST").as_deref(),
+ "/proc/self/cgroup",
+ )
+}
+
+fn is_container_indicators(
+ dockerenv_path: &Path,
+ containerenv_path: &Path,
+ container_env: Option<&OsStr>,
+ k8s_env: Option<&OsStr>,
+ cgroup_path: &str,
+) -> bool {
+ if dockerenv_path.exists() || containerenv_path.exists() {
+ return true;
}
+
+ if container_env.is_some() || k8s_env.is_some() {
+ return true;
+ }
+
+ #[cfg(target_os = "linux")]
+ {
+ if let Ok(cgroup) = std::fs::read_to_string(cgroup_path)
+ && cgroup.lines().any(|line| {
+ line.contains("/docker/")
+ || line.contains("/docker-")
+ || line.contains("/libpod-")
+ || line.contains("/podman/")
+ || line.contains("/kubepods/")
+ || line.contains("/kubepods-")
+ || line.contains("/containerd/")
+ || line.contains("/lxc/")
+ })
+ {
+ return true;
+ }
+ }
+
+ let _ = cgroup_path;
Review Comment:
nit: `let _ = cgroup_path;` does nothing on linux, where the block above
already reads the parameter. drop the parameter and read the file inside
`is_container()`.
##########
core/configs/src/server_config/validators.rs:
##########
@@ -407,29 +410,115 @@ impl ServerConfig {
/// The listener the client-facing address is derived from must not bind a
/// wildcard unless that address is declared outright.
+ ///
+ /// When running inside a container, a loopback listener means the server
+ /// is unreachable from outside the container, which is warned.
fn validate_client_facing_address(&self) -> Result<(), ConfigurationError>
{
- if self.cluster.enabled || self.node.advertised_address.is_some() {
- return Ok(());
+ self.validate_client_facing_address_in_env(is_container())?;
+ Ok(())
+ }
+
+ fn validate_client_facing_address_in_env(
+ &self,
+ is_container: bool,
+ ) -> Result<Option<String>, ConfigurationError> {
+ if self.cluster.enabled {
+ return Ok(None);
}
// No client-facing listener runs, so no client dials this node and
// there is no address to demand.
let Some(listener) = self.derived_address_listener() else {
- return Ok(());
+ return Ok(None);
};
let bind = parse_bind_address(listener.key, listener.address)?;
- if !bind.ip().to_canonical().is_unspecified() {
- return Ok(());
+ let ip = bind.ip().to_canonical();
+ if ip.is_unspecified() {
+ if self.node.advertised_address.is_none() {
+ eprintln!(
+ "{COMPONENT} - {} binds the wildcard {bind}, which says
which interfaces this node \
+ accepts on rather than where a client reaches it, so
cluster metadata would carry no \
+ address for this node. Set node.advertised_address to the
address clients dial, or \
+ bind a concrete address.",
+ listener.key
+ );
+ return Err(ConfigurationError::InvalidConfigurationValue);
+ }
+ return Ok(None);
}
- eprintln!(
- "{COMPONENT} - {} binds the wildcard {bind}, which says which
interfaces this node \
- accepts on rather than where a client reaches it, so cluster
metadata would carry no \
- address for this node. Set node.advertised_address to the address
clients dial, or \
- bind a concrete address.",
- listener.key
- );
- Err(ConfigurationError::InvalidConfigurationValue)
+ if ip.is_loopback() && is_container {
+ let env_var = format!("IGGY_{}", listener.key.replace('.',
"_").to_uppercase());
+ let port = bind.port();
+ let msg = if self.node.advertised_address.is_none() {
Review Comment:
simplification: the two `format!` arms repeat the same message and differ in
one clause, and the log happens inside the validator. build the clause once,
return the text, and log it in the caller.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
