kunaldevxxx commented on code in PR #4218:
URL: https://github.com/apache/iggy/pull/4218#discussion_r4045846695


##########
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:
   Changed to `outside this network namespace` and dropped the `0.0.0.0` hint 
when `node.advertised_address` is set, suggesting `Set {env_var} or bind a 
concrete address.` instead.



##########
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:
   Done. Now resolving the environment variable name via 
`ServerConfig::find_by_config_path(listener.key).map_or(listener.key, |m| 
m.env_name)`.



-- 
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]

Reply via email to