Simplify the host detection logic and move aws specific code into endpoints.rs

intentionally-left-nil · intentionally-left-nil · commit a0ec3d5661de · 2026-06-18T11:44:08.000-07:00
Also use from_env() to create the profile in all cases, just specifying
the profile name

Signed-off-by: Anil Kulkarni &lt;anil@terminal.space&gt;
diff --git a/crates/nono-proxy/src/aws/endpoints.rs b/crates/nono-proxy/src/aws/endpoints.rs
@@ -1,33 +1,12 @@
-//! AWS hostname parsing: service-code and region extraction.
+//! AWS endpoint parsing: extract the SigV4 region and service from an `amazonaws.com` hostname.
 //!
-//! ## Canonical AWS hostname shapes
+//! Supported hostname shapes:
+//! - Regional:   `{service}.{region}.amazonaws.com`
+//! - Global:     `{service}.amazonaws.com`
+//! - FIPS:       `{service}-fips.{region}.amazonaws.com` (`-fips` is stripped)
 //!
-//! ```text
-//! Regional:  {service}.{region}.amazonaws.com       (4 dot-segments)
-//! Global:    {service}.amazonaws.com                (3 dot-segments)
-//! FIPS:      {service}-fips.{region}.amazonaws.com  (4 dot-segments, service ends in "-fips")
-//! Dual-stack:{service}.{region}.api.aws             (different TLD — not handled here)
-//! ```
-//!
-//! `api.aws` dual-stack endpoints are out of scope for auto-detection; users
-//! on those endpoints supply explicit `aws_auth.region` and `aws_auth.service`
-//! overrides in their config.
-//!
-//! ## Region code pattern
-//!
-//! Standard regions match `^[a-z]{2,3}-[a-z]+-\d+$`.  GovCloud and ISO
-//! partition regions have an extra word between the geo prefix and direction
-//! segment (`us-gov-east-1`, `us-iso-east-1`, `us-isob-east-1`), so the
-//! effective pattern is `^[a-z]{2,3}-(?:[a-z]+-)?[a-z]+-\d+$`.  This covers
-//! all current regions including newer geo prefixes (`il-`, `mx-`, `me-`,
-//! `af-`) and high-index AZ suffixes (`ap-southeast-7`).
-//!
-//! ## Service-code table
-//!
-//! The service segment is not always the SigV4 signing name.  AWS uses
-//! irregular host prefixes (e.g. `bedrock-runtime` signs as `bedrock`,
-//! `email` signs as `ses`).  The table maps the canonical segment (with any
-//! `-fips` suffix stripped) to the signing name.
+//! `api.aws` dual-stack endpoints are not supported; supply explicit
+//! `aws_auth.region` and `aws_auth.service` config values for those.
 
 use std::collections::HashMap;
 use std::sync::OnceLock;
@@ -43,7 +22,8 @@ fn region_re() -> &'static Regex {
 }
 
 fn service_map() -> &'static HashMap<&'static str, &'static str> {
-    // Maps from the URL hostname path (key) to the AWS service name (value)
+    // Maps hostname service segment to SigV4 signing name. Some segments differ
+    // from the signing name (e.g. "bedrock-runtime" signs as "bedrock", "email" as "ses").
     static MAP: OnceLock<HashMap<&'static str, &'static str>> = OnceLock::new();
     MAP.get_or_init(|| {
         HashMap::from([
@@ -73,32 +53,18 @@ fn service_map() -> &'static HashMap<&'static str, &'static str> {
     })
 }
 
-/// Parsed components of an AWS URL
+/// Parsed components of an `amazonaws.com` hostname.
 #[derive(Debug, PartialEq, Eq)]
 pub struct AwsUrlParts<'a> {
-    /// Service-code segment from the hostname, with any `-fips` suffix
-    /// already stripped (e.g. `"bedrock-runtime"`, `"iam"`).
+    /// Service segment with any `-fips` suffix stripped (e.g. `"bedrock-runtime"`, `"iam"`).
     pub normalized_service: &'a str,
-    /// Region code if present (e.g. `"us-east-1"`), or `None` for
-    /// global-only endpoints.
+    /// Region code, or `None` for global endpoints.
     pub region: Option<&'a str>,
 }
 
 impl<'a> AwsUrlParts<'a> {
-    /// Parse a hostname into its service segment and optional region.
-    ///
-    /// Accepts only `amazonaws.com` endpoints in the two canonical shapes:
-    /// - Regional (4 segments): `{service}.{region}.amazonaws.com`
-    /// - Global   (3 segments): `{service}.amazonaws.com`
-    ///
-    /// A `-fips` suffix on the service segment is stripped before returning so
-    /// callers always receive the canonical segment name (e.g. `"bedrock-runtime"`
-    /// for both `bedrock-runtime.us-east-1.amazonaws.com` and
-    /// `bedrock-runtime-fips.us-east-1.amazonaws.com`).
-    ///
-    /// Returns `None` for non-`amazonaws.com` hosts (including `api.aws`
-    /// dual-stack endpoints), wrong segment counts, and region strings that do
-    /// not match the regex
+    /// Parse an `amazonaws.com` hostname into its service and region components.
+    /// Returns `None` for unrecognised shapes or invalid region codes.
     #[must_use]
     pub fn parse(host: &'a str) -> Option<Self> {
         match host.split('.').collect::<Vec<_>>().as_slice() {
@@ -119,12 +85,8 @@ impl<'a> AwsUrlParts<'a> {
         }
     }
 
-    /// Return the SigV4 signing service name for this endpoint.
-    ///
-    /// The `normalized_service` segment (with any `-fips` suffix already
-    /// stripped) is looked up in the service table.  Returns `None` for
-    /// unrecognised service segments; callers should fall back to an explicit
-    /// `aws_auth.service` config value or emit a warning.
+    /// Look up the SigV4 signing service name for this endpoint.
+    /// Returns `None` for unrecognised service segments.
     #[must_use]
     pub fn signing_service(&self) -> Option<&'static str> {
         service_map().get(self.normalized_service).copied()
@@ -133,25 +95,33 @@ impl<'a> AwsUrlParts<'a> {
 
 /// Resolve the SigV4 `(region, service)` pair for an AWS route.
 ///
-/// Explicit config values in `aws_auth` take precedence over auto-detection.
-/// When either is absent the `upstream_host` is parsed as an `amazonaws.com`
-/// endpoint; if the hostname is not recognised and a value cannot be inferred,
-/// returns `None` after emitting a `warn!` that names the missing field and
-/// how to fix it.
-///
-/// Returns `Some((region, service))` on success, `None` when the route should
-/// be skipped (the warning has already been emitted).
+/// Config values in `aws_auth` take precedence. When either is absent, the
+/// host of `upstream` is parsed as an `amazonaws.com` endpoint to fill in the
+/// missing value. Returns `None` and emits a warning if resolution fails.
 #[must_use]
 pub fn resolve_signing_params(
     prefix: &str,
     aws_auth: &crate::config::AwsAuthConfig,
-    upstream_host: &str,
+    upstream: &str,
 ) -> Option<(String, String)> {
-    // When either region or service is missing from config, parse the upstream
-    // host once and derive both from it. If both are explicit we skip the parse
-    // entirely.
+    let upstream_host: String;
     let parsed_host = if aws_auth.region.is_none() || aws_auth.service.is_none() {
-        let h = AwsUrlParts::parse(upstream_host);
+        upstream_host = match url::Url::parse(upstream)
+            .ok()
+            .and_then(|u| u.host_str().map(str::to_owned))
+        {
+            Some(h) => h,
+            None => {
+                warn!(
+                    "AWS route '{}': upstream '{}' is not a valid URL with a \
+                     host component; fix the upstream or set aws_auth.region \
+                     and aws_auth.service explicitly — skipping this route.",
+                    prefix, upstream
+                );
+                return None;
+            }
+        };
+        let h = AwsUrlParts::parse(&upstream_host);
         if h.is_none() {
             warn!(
                 "AWS route '{}': upstream host '{}' is not a recognised \
@@ -166,7 +136,7 @@ pub fn resolve_signing_params(
         None
     };
 
-    // Region: explicit override wins; otherwise take from parsed host.
+    // Region: explicit config wins; otherwise derive from the parsed host.
     let region = match aws_auth.region.as_deref() {
         Some(r) => r.to_owned(),
         None => match parsed_host.as_ref().and_then(|h| h.region) {
@@ -176,15 +146,14 @@ pub fn resolve_signing_params(
                     "AWS route '{}': could not determine region from upstream \
                      host '{}' (global endpoint?); set aws_auth.region \
                      explicitly — skipping this route.",
-                    prefix, upstream_host
+                    prefix, upstream
                 );
                 return None;
             }
         },
     };
 
-    // Service: explicit override wins; otherwise look up signing name from the
-    // parsed host's service segment.
+    // Service: explicit config wins; otherwise look up from the parsed host.
     let service = match aws_auth.service.as_deref() {
         Some(s) => s.to_owned(),
         None => {
@@ -199,7 +168,7 @@ pub fn resolve_signing_params(
                         "AWS route '{}': service segment '{}' from upstream \
                          host '{}' is not in the signing-name table; set \
                          aws_auth.service explicitly — skipping this route.",
-                        prefix, seg, upstream_host
+                        prefix, seg, upstream
                     );
                     return None;
                 }
diff --git a/crates/nono-proxy/src/aws/route.rs b/crates/nono-proxy/src/aws/route.rs
@@ -7,8 +7,6 @@
 
 use std::collections::HashMap;
 
-use aws_config::BehaviorVersion;
-use aws_config::profile::ProfileFileCredentialsProvider;
 use aws_credential_types::provider::SharedCredentialsProvider;
 use tracing::debug;
 
@@ -131,42 +129,24 @@ impl AwsRouteTable {
     }
 }
 
-/// Build a fresh `SharedCredentialsProvider` for the given profile.
-///
-/// - `Some(name)`: uses `ProfileFileCredentialsProvider` for the named profile.
-/// - `None`: uses the SDK default credential chain.
+/// Build a `SharedCredentialsProvider` for the given profile, or the default
+/// credential chain if `profile` is `None`.
 async fn new_provider(profile: Option<&str>) -> Result<SharedCredentialsProvider, String> {
-    // Behavior version is pinned to avoid silent breaking changes from latest().
-    let behavior = BehaviorVersion::v2026_01_12();
+    debug!("aws::route: building provider for profile={:?}", profile);
 
-    let provider = match profile {
-        Some(name) => {
-            debug!(
-                "aws::route: building ProfileFileCredentialsProvider for profile={:?}",
-                name
-            );
-            SharedCredentialsProvider::new(
-                ProfileFileCredentialsProvider::builder()
-                    .profile_name(name)
-                    .build(),
-            )
-        }
-        None => {
-            debug!("aws::route: building default credential chain provider");
-            let sdk_config = aws_config::from_env()
-                .behavior_version(behavior)
-                .load()
-                .await;
-            sdk_config
-                .credentials_provider()
-                .ok_or_else(|| {
-                    "default AWS credential chain yielded no provider; \
-                     set AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY or configure an AWS profile"
-                        .to_string()
-                })?
-                .clone()
-        }
-    };
+    let mut loader = aws_config::from_env();
+    if let Some(name) = profile {
+        loader = loader.profile_name(name);
+    }
 
+    let sdk_config = loader.load().await;
+    let provider = sdk_config
+        .credentials_provider()
+        .ok_or_else(|| {
+            "default AWS credential chain yielded no provider; \
+             set AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY or configure an AWS profile"
+                .to_string()
+        })?
+        .clone();
     Ok(provider)
 }
diff --git a/crates/nono-proxy/src/credential.rs b/crates/nono-proxy/src/credential.rs
@@ -301,7 +301,6 @@ impl CredentialStore {
                     }
                 }
             } else if let Some(ref aws_auth) = route.aws_auth {
-                let upstream_host = extract_host(&route.upstream);
                 debug!(
                     "aws credential load: prefix='{}' upstream='{}' \
                      explicit_region={:?} explicit_service={:?} profile={:?}",
@@ -315,7 +314,7 @@ impl CredentialStore {
                 let Some((region, service)) = crate::aws::endpoints::resolve_signing_params(
                     &normalized_prefix,
                     aws_auth,
-                    &upstream_host,
+                    &route.upstream,
                 ) else {
                     continue;
                 };
@@ -438,16 +437,6 @@ const KEYRING_SERVICE: &str = nono::keystore::DEFAULT_SERVICE;
 
 const KEYRING_TIMEOUT_HINT: &str = " Set NONO_KEYRING_TIMEOUT_SECS=N (default 120) to wait longer for keychain unlock; 0 disables the timeout.";
 
-/// Extract the hostname from a URL string, returning an empty string on parse
-/// failure. Used to derive the AWS region and service from the upstream URL at
-/// credential load time.
-fn extract_host(url: &str) -> String {
-    url::Url::parse(url)
-        .ok()
-        .and_then(|u| u.host_str().map(|h| h.to_string()))
-        .unwrap_or_default()
-}
-
 /// Redact a credential reference for safe display in warnings.
 ///
 /// Delegates to the appropriate URI-specific redaction helper so that
