refactor(cli): extract allowed-ip option parsing

Signed-off-by: Adrien Langou <alangou@nvidia.com>

Author: alangou

crates/openshell-cli/src/policy_update.rs

@@ -351,6 +351,8 @@ fn parse_add_endpoint_spec(spec: &str) -> Result<NetworkEndpoint> {
     Ok(endpoint)
 }
 
+const ALLOWED_IP_OPTION_PREFIX: &str = "allowed-ip=";
+
 fn apply_add_endpoint_options(
     spec: &str,
     endpoint: &mut NetworkEndpoint,
@@ -379,37 +381,40 @@ fn apply_add_endpoint_options(
                 ensure_request_body_credential_rewrite_protocol(spec, endpoint)?;
                 endpoint.request_body_credential_rewrite = true;
             }
-            _ => {
-                let Some(allowed_ip) = option.strip_prefix("allowed-ip=") else {
-                    return Err(miette!(
-                        "--add-endpoint options segment supports only 'allow-uninspected-credentials', 'websocket-credential-rewrite', 'request-body-credential-rewrite', and 'allowed-ip=<CIDR-or-IP>'; got '{option}' in '{spec}'"
-                    ));
-                };
-                let allowed_ip = allowed_ip.trim();
-                if allowed_ip.is_empty() {
-                    return Err(miette!(
-                        "--add-endpoint allowed-ip option must include a CIDR or IP value in '{spec}'"
-                    ));
-                }
-                if allowed_ip.contains(char::is_whitespace) {
-                    return Err(miette!(
-                        "--add-endpoint allowed-ip option must not contain whitespace in '{spec}'"
-                    ));
-                }
-                if !endpoint
-                    .allowed_ips
-                    .iter()
-                    .any(|existing| existing == allowed_ip)
-                {
-                    endpoint.allowed_ips.push(allowed_ip.to_string());
+            _ if option.starts_with(ALLOWED_IP_OPTION_PREFIX) => {
+                let allowed_ip =
+                    parse_allowed_ip_value(spec, &option[ALLOWED_IP_OPTION_PREFIX.len()..])?;
+                if !endpoint.allowed_ips.contains(&allowed_ip) {
+                    endpoint.allowed_ips.push(allowed_ip);
                 }
             }
+            _ => {
+                return Err(miette!(
+                    "--add-endpoint options segment supports only 'allow-uninspected-credentials', 'websocket-credential-rewrite', 'request-body-credential-rewrite', and 'allowed-ip=<CIDR-or-IP>'; got '{option}' in '{spec}'"
+                ));
+            }
         }
     }
 
     Ok(())
 }
 
+/// Validate the value part of an `allowed-ip=<CIDR-or-IP>` endpoint option.
+fn parse_allowed_ip_value(spec: &str, value: &str) -> Result<String> {
+    let allowed_ip = value.trim();
+    if allowed_ip.is_empty() {
+        return Err(miette!(
+            "--add-endpoint allowed-ip option must include a CIDR or IP value in '{spec}'"
+        ));
+    }
+    if allowed_ip.contains(char::is_whitespace) {
+        return Err(miette!(
+            "--add-endpoint allowed-ip option must not contain whitespace in '{spec}'"
+        ));
+    }
+    Ok(allowed_ip.to_string())
+}
+
 fn parse_host(flag: &str, spec: &str, host: &str) -> Result<String> {
     let host = host.trim();
     if host.is_empty() {
@@ -457,6 +462,7 @@ fn dedup_strings(values: &[String]) -> Vec<String> {
 mod tests {
     use super::{
         PolicyUpdatePlan, build_policy_update_plan as build_policy_update_plan_with_options,
+        parse_allowed_ip_value,
     };
     use openshell_policy::PolicyMergeOp;
 
@@ -687,6 +693,28 @@ mod tests {
         assert!(error.to_string().contains("allowed-ip option"));
     }
 
+    #[test]
+    fn parse_allowed_ip_value_accepts_trimmed_cidr_and_ip() {
+        assert_eq!(
+            parse_allowed_ip_value("spec", "10.0.0.0/8").expect("CIDR should parse"),
+            "10.0.0.0/8"
+        );
+        assert_eq!(
+            parse_allowed_ip_value("spec", "  192.168.1.10  ").expect("IP should parse"),
+            "192.168.1.10"
+        );
+    }
+
+    #[test]
+    fn parse_allowed_ip_value_rejects_empty_and_interior_whitespace() {
+        let empty = parse_allowed_ip_value("spec", "   ").expect_err("empty value must fail");
+        assert!(empty.to_string().contains("must include a CIDR or IP"));
+
+        let spaced =
+            parse_allowed_ip_value("spec", "10.0.0.0/8 172.16.0.0/12").expect_err("must fail");
+        assert!(spaced.to_string().contains("must not contain whitespace"));
+    }
+
     #[test]
     fn websocket_credential_rewrite_rejects_l4_endpoint() {
         let error = build_policy_update_plan(

crates/openshell-server/src/grpc/policy.rs

@@ -52,7 +52,7 @@ use openshell_core::telemetry::{
 use openshell_core::{
     VERSION,
     endpoint_path::EndpointPathPattern,
-    host_pattern::host_matches,
+    host_pattern::{host_matches, host_patterns_overlap},
     settings::{self, SettingValueKind},
 };
 use openshell_ocsf::{
@@ -2277,42 +2277,11 @@ fn endpoint_ports(endpoint: &NetworkEndpoint) -> Vec<u32> {
     }
 }
 
-fn host_patterns_overlap(left: &str, right: &str) -> bool {
-    if left.eq_ignore_ascii_case(right) {
-        return true;
-    }
-
-    let left = left.to_ascii_lowercase();
-    let right = right.to_ascii_lowercase();
-    let left_has_wildcard = left.contains('*');
-    let right_has_wildcard = right.contains('*');
-
-    if !right_has_wildcard {
-        return host_matches(&left, &right).unwrap_or(false);
-    }
-    if !left_has_wildcard {
-        return host_matches(&right, &left).unwrap_or(false);
-    }
-
-    fn literal_suffix(pattern: &str) -> &str {
-        pattern
-            .rfind('*')
-            .map_or(pattern, |index| &pattern[index + 1..])
-    }
-
-    let left_suffix = literal_suffix(&left);
-    let right_suffix = literal_suffix(&right);
-    left_suffix.is_empty()
-        || right_suffix.is_empty()
-        || left_suffix.ends_with(right_suffix)
-        || right_suffix.ends_with(left_suffix)
-}
-
 fn endpoint_matches_credentialed_scope(
     endpoint: &NetworkEndpoint,
     scope: &CredentialedEndpointScope,
 ) -> bool {
-    if !host_patterns_overlap(&endpoint.host, &scope.host) {
+    if !host_patterns_overlap(&endpoint.host, &scope.host).unwrap_or(false) {
         return false;
     }
     let endpoint_ports = endpoint_ports(endpoint);
@@ -5790,6 +5759,12 @@ mod tests {
                             provider_credentialed: true,
                             ..Default::default()
                         },
+                        NetworkEndpoint {
+                            host: "*.api.example.com".to_string(),
+                            port: 443,
+                            provider_credentialed: true,
+                            ..Default::default()
+                        },
                     ],
                     ..Default::default()
                 },
@@ -5807,6 +5782,7 @@ mod tests {
         let endpoints = &policy.network_policies["test"].endpoints;
         assert!(endpoints[0].provider_credentialed);
         assert!(!endpoints[1].provider_credentialed);
+        assert!(!endpoints[2].provider_credentialed);
     }
 
     #[test]