From dc3b06ee5c37001bd89d2232bf562586221aada8 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Mon, 17 Aug 2026 12:59:29 -0700 Subject: [PATCH 1/2] refactor(environments): split mod.rs into config/devx_core/catalog submodules rust/src/environments/mod.rs was approaching the 1000-line CI limit, blocking contributors adding to it in an open PR and another branch. Splits it by responsibility: GddyEnvConfig + URL resolution helpers (config.rs), DevX Core gateway URL resolution (devx_core.rs), and API-catalog domain base-URL resolution (catalog.rs), with each submodule's tests moving alongside it. mod.rs keeps only the compiled-in registry, the shared Environments instance, and the public resolve/listable/is_known API. Public API is unchanged via re-exports. Co-Authored-By: Claude Sonnet 5 --- rust/src/environments/catalog.rs | 106 ++++ rust/src/environments/config.rs | 503 ++++++++++++++++++ rust/src/environments/devx_core.rs | 79 +++ rust/src/environments/mod.rs | 699 +------------------------- rust/src/environments/test_support.rs | 43 ++ 5 files changed, 743 insertions(+), 687 deletions(-) create mode 100644 rust/src/environments/catalog.rs create mode 100644 rust/src/environments/config.rs create mode 100644 rust/src/environments/devx_core.rs create mode 100644 rust/src/environments/test_support.rs diff --git a/rust/src/environments/catalog.rs b/rust/src/environments/catalog.rs new file mode 100644 index 00000000..6302a449 --- /dev/null +++ b/rust/src/environments/catalog.rs @@ -0,0 +1,106 @@ +//! Environment-specific base-URL resolution for API-catalog domains (used by +//! `api domain list` / `api call`). + +use cli_engine::ConfigSource; + +use super::config::{clean_url, substitute_env_host}; +use super::{DEFAULT_ENV, env_prefix, instance}; + +/// Resolve the environment-specific base URL for a catalog domain (used by +/// `api domain list` / `api call`). Checks an explicit override first — a +/// `_api_url` key from local config, or a `__API_URL` +/// env var read directly here (outside `GddyEnvConfig`'s own fields entirely, +/// since a per-domain override key isn't one of them). Checking the env var +/// directly makes this work uniformly for `prod`/`ote` builtins and custom +/// dev/test env names alike. Absent an override, falls back to GoDaddy's +/// `{env}-godaddy.com` hostname convention against `prod_base_url` for any +/// non-`prod` environment. +pub fn resolve_catalog_base_url(domain: &str, prod_base_url: &str, env_name: &str) -> String { + let override_key = format!("{}_api_url", domain.replace('-', "_")); + if let Some(url) = domain_override(&override_key, env_name, |k| std::env::var(k).ok()) { + return url; + } + if env_name == DEFAULT_ENV { + return prod_base_url.to_owned(); + } + substitute_env_host(prod_base_url, env_name).unwrap_or_else(|| prod_base_url.to_owned()) +} + +/// Pure lookup for [`resolve_catalog_base_url`]'s override, with the env-var +/// getter injected so tests stay parallel-safe (no real `std::env::set_var`). +/// `instance().source(env_name)` errors for an environment unknown to every +/// compiled/file layer, which this treats the same as "no local config +/// entry" — falling through to the env var — rather than propagating. +fn domain_override( + override_key: &str, + env_name: &str, + var: impl Fn(&str) -> Option, +) -> Option { + if let Ok(source) = instance().source(env_name) + && let Some(raw) = source + .toml_value(override_key) + .and_then(toml::Value::as_str) + && let Some(clean) = clean_url(raw) + { + return Some(clean); + } + let var_name = format!("{}_{}", env_prefix(env_name), override_key.to_uppercase()); + var(&var_name).and_then(|v| clean_url(&v)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn domain_override_falls_back_to_env_var_when_no_local_config_entry() { + // "fulfillments-catalog-test" is not a compiled builtin and has no + // local config entry, so the first (local-config) branch misses and + // the injected var getter is consulted directly. + let var = |k: &str| { + (k == "FULFILLMENTS_CATALOG_TEST_FULFILLMENTS_API_URL") + .then(|| "https://fulfillments.example.test".to_owned()) + }; + let resolved = domain_override("fulfillments_api_url", "fulfillments-catalog-test", var); + assert_eq!( + resolved, + Some("https://fulfillments.example.test".to_owned()) + ); + } + + #[test] + fn domain_override_is_none_when_neither_layer_has_it() { + let resolved = domain_override("fulfillments_api_url", "fulfillments-catalog-test", |_| { + None + }); + assert_eq!(resolved, None); + } + + #[test] + fn resolve_catalog_base_url_returns_prod_unchanged() { + let url = resolve_catalog_base_url( + "fulfillments", + "https://fulfillment.api.commerce.godaddy.com/v1/commerce", + "prod", + ); + assert_eq!( + url, + "https://fulfillment.api.commerce.godaddy.com/v1/commerce" + ); + } + + #[test] + fn resolve_catalog_base_url_applies_convention_for_non_prod() { + // No override exists anywhere for this made-up env/domain pair, so + // this exercises the `{env}-godaddy.com` convention fallback. + let url = resolve_catalog_base_url( + "fulfillments", + "https://fulfillment.api.commerce.godaddy.com/v1/commerce", + "ote", + ); + assert_eq!( + url, + "https://fulfillment.api.commerce.ote-godaddy.com/v1/commerce" + ); + } +} diff --git a/rust/src/environments/config.rs b/rust/src/environments/config.rs new file mode 100644 index 00000000..29c63233 --- /dev/null +++ b/rust/src/environments/config.rs @@ -0,0 +1,503 @@ +//! `GddyEnvConfig`: the fully-resolved environment config assembled by +//! cli-engine's `EnvConfig` derive from compiled-in defaults, an +//! `environments.toml` file, and `GDDY_*` env var overrides — plus the URL +//! validation and GoDaddy hostname-convention helpers its fields lean on. + +use cli_engine::{EnvConfig, SourceChain}; + +/// A fully-resolved environment config +#[derive(Debug, Clone, Default, EnvConfig)] +pub struct GddyEnvConfig { + #[env_config(default_fn = default_name)] + pub name: String, + + #[env_config(from_toml = parse_url_from_toml)] + pub api_url: String, + + /// No default: every environment must supply a real OAuth client id. + pub client_id: String, + + /// Overridable at runtime via `GDDY_AUTH_URL` — e.g. to point at a local + /// dev auth server without editing `environments.toml`. + #[env_config( + from_toml = parse_url_from_toml, + env = "AUTH_URL", + from_env = parse_url, + default_fn = default_auth_url + )] + pub auth_url: String, + + /// Overridable at runtime via `GDDY_TOKEN_URL`. + #[env_config( + from_toml = parse_url_from_toml, + env = "TOKEN_URL", + from_env = parse_url, + default_fn = default_token_url + )] + pub token_url: String, + + /// Base URL for the domain commands. Some endpoints (e.g. domain + /// availability) live behind a different host than the OAuth/`api_url` + /// service; this defaults to `api_url` when not overridden. Overridable + /// at runtime via `GDDY_DOMAINS_API_URL`. + #[env_config( + from_toml = parse_url_from_toml, + env = "DOMAINS_API_URL", + from_env = parse_url, + default_fn = default_domains_api_url + )] + pub domains_api_url: String, + + /// Base URL for the account management site (e.g. adding payment methods). + /// Defaults to `account.godaddy.com` for prod and `account.{env}-godaddy.com` + /// for other environments; overridable via `GDDY_ACCOUNT_URL` or local config. + #[env_config( + from_toml = parse_url_from_toml, + env = "ACCOUNT_URL", + from_env = parse_url, + default_fn = default_account_url + )] + pub account_url: String, +} + +/// `name`'s `default_fn`: the field itself is never set by any real TOML/env +/// source, so this fires unconditionally, reading the environment's own +/// identity off the chain instead of a sibling field's value. +fn default_name(sources: &SourceChain<'_>) -> String { + sources.env_name().unwrap_or_default().to_owned() +} + +/// The already-resolved, cleaned `api_url` for this chain — same value the +/// `api_url` field itself holds, re-derived from the raw chain rather than +/// `Self` (a `default_fn` only ever sees the chain, not sibling fields +/// already computed on the struct being built). Declaring `api_url` before +/// the fields that call this, so their own resolution only runs once +/// `api_url`'s already succeeded, is what makes re-deriving from the raw +/// value safe — a malformed `api_url` fails the whole `assemble` before any +/// of these `default_fn`s could run. +fn current_api_url(sources: &SourceChain<'_>) -> String { + sources + .toml_value("api_url") + .and_then(toml::Value::as_str) + .and_then(clean_url) + .unwrap_or_default() +} + +fn default_auth_url(sources: &SourceChain<'_>) -> String { + derive_auth_url(¤t_api_url(sources)) +} + +fn default_token_url(sources: &SourceChain<'_>) -> String { + derive_token_url(¤t_api_url(sources)) +} + +fn default_domains_api_url(sources: &SourceChain<'_>) -> String { + current_api_url(sources) +} + +fn default_account_url(sources: &SourceChain<'_>) -> String { + derive_account_url(sources.env_name().unwrap_or_default()) +} + +fn derive_account_url(env_name: &str) -> String { + if env_name == "prod" { + return "https://account.godaddy.com".to_owned(); + } + substitute_env_host("https://account.godaddy.com", env_name) + .unwrap_or_else(|| "https://account.godaddy.com".to_owned()) +} + +/// Applies GoDaddy's internal-environment hostname convention +/// (`{env}-godaddy.com`) to a canonical `*.godaddy.com` URL, preserving any +/// subdomain prefix and the original scheme/path. Returns `None` if the +/// host isn't under `godaddy.com` — nothing to substitute. +/// +/// `pub(super)`: also used by [`super::catalog::resolve_catalog_base_url`]. +pub(super) fn substitute_env_host(url: &str, env_name: &str) -> Option { + let lower = url.to_ascii_lowercase(); + let scheme_len = if lower.starts_with("https://") { + 8 + } else if lower.starts_with("http://") { + 7 + } else { + return None; + }; + let rest = &url[scheme_len..]; + let host_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); + let host = &rest[..host_end]; + let host_lower = host.to_ascii_lowercase(); + let new_host = if host_lower == "godaddy.com" { + format!("{env_name}-godaddy.com") + } else { + let prefix = host_lower.strip_suffix(".godaddy.com")?; + format!("{}.{env_name}-godaddy.com", &host[..prefix.len()]) + }; + Some(format!( + "{}{}{}", + &url[..scheme_len], + new_host, + &rest[host_end..] + )) +} + +fn derive_auth_url(api_url: &str) -> String { + format!("{}/v2/oauth2/authorize", api_url.trim_end_matches('/')) +} + +fn derive_token_url(api_url: &str) -> String { + format!("{}/v2/oauth2/token", api_url.trim_end_matches('/')) +} + +/// Validates and normalizes a candidate URL. Trims surrounding +/// whitespace and any trailing slash, and requires an `http(s)://` scheme with a +/// non-empty host (reqwest needs an absolute URL). Returns `None` for an +/// empty/whitespace or schemeless value, so a blank or malformed value never +/// resolves to a relative/unusable URL. +/// +/// `pub(super)`: also used by [`super::catalog::domain_override`] and +/// [`super::devx_core::devx_core_url_with`]. +pub(super) fn clean_url(raw: &str) -> Option { + let trimmed = raw.trim().trim_end_matches('/'); + // Require an http(s):// scheme (case-insensitive per RFC 3986, so `HTTPS://` + // is valid) and a non-empty host. The host is the segment before any + // path/query/fragment, so this rejects `https:///path`, `https://`, and + // `https://?x` (which a lenient URL parser would accept). + let lower = trimmed.to_ascii_lowercase(); + let scheme_len = if lower.starts_with("https://") { + "https://".len() + } else if lower.starts_with("http://") { + "http://".len() + } else { + return None; + }; + let host = trimmed[scheme_len..] + .split(['/', '?', '#']) + .next() + .unwrap_or(""); + (!host.is_empty()).then(|| trimmed.to_owned()) +} + +/// Validates a candidate URL string. `EnvConfig` `from_env` shared by every +/// env-var-overridable URL field — an env var is already a plain `&str`, so +/// this is the direct validator with nothing to unwrap first. Blank values +/// never reach this — every `EnvConfig` field treats a blank source answer +/// as absent by default; a non-blank value must be a real http(s) URL. +fn parse_url(raw: &str) -> Result { + clean_url(raw).ok_or_else(|| format!("{raw:?} is not a valid http(s) URL")) +} + +/// `EnvConfig` `from_toml` shared by every URL field — a TOML value carries +/// its own type, so this checks it's actually a string before delegating to +/// [`parse_url`], the shared core every URL field's `from_env` also uses +/// directly. +fn parse_url_from_toml(value: &toml::Value) -> Result { + let raw = value + .as_str() + .ok_or_else(|| "expected a string".to_owned())?; + parse_url(raw) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::environments::APP_ID; + use crate::environments::test_support::{ENV_LOCK, EnvGuard}; + use cli_engine::environments::{EnvTable, Environments}; + + fn test_environment(name: &str, extend: impl FnOnce(EnvTable) -> EnvTable) -> GddyEnvConfig { + Environments::new(name) + .with_environment(name, extend(EnvTable::new())) + .resolve(name) + .expect("resolves") + } + + #[test] + fn resolved_env_derives_oauth_urls_from_api_url_when_unset() { + let resolved = test_environment("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + }); + assert_eq!(resolved.name, "dev"); + assert_eq!(resolved.api_url, "https://api.example.test"); + assert_eq!(resolved.client_id, "cid"); + assert_eq!( + resolved.auth_url, + "https://api.example.test/v2/oauth2/authorize" + ); + assert_eq!( + resolved.token_url, + "https://api.example.test/v2/oauth2/token" + ); + } + + #[test] + fn resolved_env_prefers_explicit_oauth_urls_over_derived() { + let resolved = test_environment("dev", |t| { + t.with("client_id", "cid") + .with("auth_url", "https://auth.example.test/authorize") + .with("token_url", "https://auth.example.test/token") + .with("api_url", "https://api.example.test") + }); + assert_eq!(resolved.auth_url, "https://auth.example.test/authorize"); + assert_eq!(resolved.token_url, "https://auth.example.test/token"); + } + + #[test] + fn resolved_env_falls_back_to_derived_oauth_urls_when_override_is_blank() { + // A blank `auth_url` (from a TOML value here, or from `GDDY_AUTH_URL=" "` + // — see `blank_env_var_override_falls_through_to_derived_auth_url`) is + // treated the same as unset. A genuinely malformed (non-blank) override + // is a hard resolve error instead — see + // `register_rejects_a_malformed_file_layer_auth_url_override_for_a_builtin`. + let resolved = test_environment("dev", |t| { + t.with("client_id", "cid") + .with("auth_url", " ") + .with("api_url", "https://api.example.test") + }); + assert_eq!( + resolved.auth_url, + "https://api.example.test/v2/oauth2/authorize" + ); + } + + #[test] + fn resolve_rejects_missing_api_url() { + let err = Environments::new("dev") + .with_environment("dev", EnvTable::new().with("client_id", "cid")) + .resolve::("dev") + .expect_err("no api_url"); + assert!(err.to_string().contains("api_url")); + } + + #[test] + fn resolve_rejects_missing_client_id() { + // client_id has no default: every environment (built-in, file, or + // hand-built for a test) must supply a real one. + let err = Environments::new("dev") + .with_environment( + "dev", + EnvTable::new().with("api_url", "https://api.example.test"), + ) + .resolve::("dev") + .expect_err("no client_id"); + assert!(err.to_string().contains("client_id")); + } + + #[test] + fn resolve_rejects_blank_api_url_final_value() { + // Unlike auth_url/token_url/domains_api_url/account_url, api_url has + // no sensible derived fallback, so blank is rejected (as `MissingField`, + // since a blank source answer is treated as absent by default). + let err = Environments::new("dev") + .with_environment( + "dev", + EnvTable::new() + .with("client_id", "cid") + .with("api_url", " "), + ) + .resolve::("dev") + .expect_err("blank api_url must be rejected"); + assert!(err.to_string().contains("api_url")); + } + + #[test] + fn domains_api_url_defaults_to_api_url() { + let resolved = test_environment("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + }); + assert_eq!(resolved.domains_api_url, resolved.api_url); + } + + #[test] + fn domains_api_url_override_is_respected() { + let resolved = test_environment("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + .with("domains_api_url", "https://domains.example.test") + }); + assert_eq!(resolved.domains_api_url, "https://domains.example.test"); + } + + #[test] + fn account_url_defaults_to_bare_domain_for_prod() { + let resolved = test_environment("prod", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.godaddy.com") + }); + assert_eq!(resolved.account_url, "https://account.godaddy.com"); + } + + #[test] + fn account_url_defaults_to_prefixed_domain_for_non_prod() { + let resolved = test_environment("ote", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.ote-godaddy.com") + }); + assert_eq!(resolved.account_url, "https://account.ote-godaddy.com"); + } + + #[test] + fn account_url_override_is_respected() { + let resolved = test_environment("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + .with("account_url", "https://account.override.test") + }); + assert_eq!(resolved.account_url, "https://account.override.test"); + } + + fn test_environment_with_app_id( + name: &str, + extend: impl FnOnce(EnvTable) -> EnvTable, + ) -> GddyEnvConfig { + Environments::new(name) + .with_app_id(APP_ID) + .with_environment(name, extend(EnvTable::new())) + .resolve(name) + .expect("resolves") + } + + #[test] + fn env_var_overrides_auth_url() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvGuard::set("GDDY_AUTH_URL", "https://auth.override.test"); + + let resolved = test_environment_with_app_id("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + .with("auth_url", "https://auth.example.test/authorize") + }); + assert_eq!( + resolved.auth_url, "https://auth.override.test", + "env var must win over the TOML value" + ); + } + + #[test] + fn env_var_overrides_token_url() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvGuard::set("GDDY_TOKEN_URL", "https://token.override.test"); + + let resolved = test_environment_with_app_id("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + }); + assert_eq!(resolved.token_url, "https://token.override.test"); + } + + #[test] + fn env_var_overrides_domains_api_url() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvGuard::set("GDDY_DOMAINS_API_URL", "https://domains.override.test"); + + let resolved = test_environment_with_app_id("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + }); + assert_eq!(resolved.domains_api_url, "https://domains.override.test"); + } + + #[test] + fn env_var_overrides_account_url() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvGuard::set("GDDY_ACCOUNT_URL", "https://account.override.test"); + + let resolved = test_environment_with_app_id("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + }); + assert_eq!(resolved.account_url, "https://account.override.test"); + } + + #[test] + fn substitute_env_host_prefixes_a_bare_domain() { + assert_eq!( + substitute_env_host("https://godaddy.com", "ote"), + Some("https://ote-godaddy.com".to_owned()) + ); + } + + #[test] + fn substitute_env_host_preserves_subdomain_and_path() { + assert_eq!( + substitute_env_host( + "https://fulfillment.api.commerce.godaddy.com/v1/commerce", + "dev" + ), + Some("https://fulfillment.api.commerce.dev-godaddy.com/v1/commerce".to_owned()) + ); + } + + #[test] + fn substitute_env_host_returns_none_for_non_godaddy_host() { + assert_eq!(substitute_env_host("https://example.com/v1", "ote"), None); + } + + #[test] + fn env_var_override_rejects_a_malformed_url() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvGuard::set("GDDY_AUTH_URL", "not-a-url"); + + let err = Environments::new("dev") + .with_app_id(APP_ID) + .with_environment( + "dev", + EnvTable::new() + .with("client_id", "cid") + .with("api_url", "https://api.example.test"), + ) + .resolve::("dev") + .expect_err("a malformed env var override must be a hard error"); + assert!(err.to_string().contains("auth_url")); + } + + #[test] + fn blank_env_var_override_falls_through_to_derived_auth_url() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvGuard::set("GDDY_AUTH_URL", " "); + + let resolved = test_environment_with_app_id("dev", |t| { + t.with("client_id", "cid") + .with("api_url", "https://api.example.test") + }); + assert_eq!( + resolved.auth_url, "https://api.example.test/v2/oauth2/authorize", + "a blank env var override is treated as absent, same as a blank TOML value" + ); + } + + #[test] + fn clean_url_requires_a_non_empty_host() { + assert_eq!( + clean_url("https://api.example.test/"), + Some("https://api.example.test".to_owned()) + ); + assert!(clean_url("https:///path").is_none()); + assert!(clean_url("https://").is_none()); + assert!(clean_url("https://?x").is_none()); + assert!(clean_url("ftp://x").is_none()); + assert!(clean_url("api.example.test").is_none()); + assert!(clean_url("not a url").is_none()); + assert_eq!( + clean_url("HTTPS://api.Example.test"), + Some("HTTPS://api.Example.test".to_owned()) + ); + assert_eq!( + clean_url("http://localhost:8080/api/"), + Some("http://localhost:8080/api".to_owned()) + ); + } +} diff --git a/rust/src/environments/devx_core.rs b/rust/src/environments/devx_core.rs new file mode 100644 index 00000000..6991f1f7 --- /dev/null +++ b/rust/src/environments/devx_core.rs @@ -0,0 +1,79 @@ +//! DevX Core API gateway base-URL resolution per environment. + +use super::config::clean_url; +use super::env_prefix; + +/// DevX Core API gateway base URL for each compiled-in builtin, consulted by +/// [`devx_core_url_with`] only after both env-var override tiers miss. +const BUILTIN_DEVX_CORE_URLS: &[(&str, &str)] = &[ + ("ote", "https://api.developer.commerce.ote-godaddy.com"), + ("prod", "https://api.developer.commerce.godaddy.com"), +]; + +/// Base URL for the DevX Core API gateway for the given environment. +/// +/// Custom environments must set `_DEVX_CORE_URL` (for example, +/// `DEV_DEVX_CORE_URL`) or the global `DEVX_CORE_URL`. `prod` and `ote` use +/// their compiled-in endpoints unless either variable overrides them. +pub fn devx_core_url(name: &str) -> Option { + devx_core_url_with(name, |key| std::env::var(key).ok()) +} + +fn devx_core_url_with(name: &str, var: impl Fn(&str) -> Option) -> Option { + let prefix = env_prefix(name); + var(&format!("{prefix}_DEVX_CORE_URL")) + .and_then(|value| clean_url(&value)) + .or_else(|| var("DEVX_CORE_URL").and_then(|value| clean_url(&value))) + .or_else(|| { + BUILTIN_DEVX_CORE_URLS + .iter() + .find(|(n, _)| *n == name) + .map(|(_, url)| (*url).to_owned()) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn devx_core_url_uses_prod_and_ote_builtins() { + assert_eq!( + devx_core_url_with("prod", |_| None).as_deref(), + Some("https://api.developer.commerce.godaddy.com") + ); + assert_eq!( + devx_core_url_with("ote", |_| None).as_deref(), + Some("https://api.developer.commerce.ote-godaddy.com") + ); + } + + #[test] + fn devx_core_url_global_override_wins() { + assert_eq!( + devx_core_url_with("prod", |key| { + (key == "DEVX_CORE_URL").then(|| " http://localhost:4000/ ".to_owned()) + }) + .as_deref(), + Some("http://localhost:4000") + ); + } + + #[test] + fn devx_core_url_per_environment_override_wins_over_global() { + assert_eq!( + devx_core_url_with("dev", |key| match key { + "DEV_DEVX_CORE_URL" => Some("https://dev-core.example.test/".to_owned()), + "DEVX_CORE_URL" => Some("https://shared-core.example.test".to_owned()), + _ => None, + }) + .as_deref(), + Some("https://dev-core.example.test") + ); + } + + #[test] + fn devx_core_url_custom_env_requires_override() { + assert_eq!(devx_core_url_with("dev", |_| None), None); + } +} diff --git a/rust/src/environments/mod.rs b/rust/src/environments/mod.rs index 0e3e0313..43e24bbb 100644 --- a/rust/src/environments/mod.rs +++ b/rust/src/environments/mod.rs @@ -24,10 +24,20 @@ //! "some-flag-key" = "beta" //! ``` +mod catalog; +mod config; +mod devx_core; +#[cfg(test)] +mod test_support; + use std::sync::{Arc, LazyLock, OnceLock}; +use cli_engine::EnvConfig; use cli_engine::environments::Environments; -use cli_engine::{ConfigSource, EnvConfig, SourceChain}; + +pub use catalog::resolve_catalog_base_url; +pub use config::GddyEnvConfig; +pub use devx_core::devx_core_url; pub const DEFAULT_ENV: &str = "prod"; @@ -67,268 +77,10 @@ pub const DEFAULT_OAUTH_SCOPES: &[&str] = &[ pub const REDIRECT_URI: &str = "http://localhost:7443/callback"; pub const APP_ID: &str = "gddy"; -/// DevX Core API gateway base URL for each compiled-in builtin, consulted by -/// [`devx_core_url_with`] only after both env-var override tiers miss. -const BUILTIN_DEVX_CORE_URLS: &[(&str, &str)] = &[ - ("ote", "https://api.developer.commerce.ote-godaddy.com"), - ("prod", "https://api.developer.commerce.godaddy.com"), -]; - -/// A fully-resolved environment config -#[derive(Debug, Clone, Default, EnvConfig)] -pub struct GddyEnvConfig { - #[env_config(default_fn = default_name)] - pub name: String, - - #[env_config(from_toml = parse_url_from_toml)] - pub api_url: String, - - /// No default: every environment must supply a real OAuth client id. - pub client_id: String, - - /// Overridable at runtime via `GDDY_AUTH_URL` — e.g. to point at a local - /// dev auth server without editing `environments.toml`. - #[env_config( - from_toml = parse_url_from_toml, - env = "AUTH_URL", - from_env = parse_url, - default_fn = default_auth_url - )] - pub auth_url: String, - - /// Overridable at runtime via `GDDY_TOKEN_URL`. - #[env_config( - from_toml = parse_url_from_toml, - env = "TOKEN_URL", - from_env = parse_url, - default_fn = default_token_url - )] - pub token_url: String, - - /// Base URL for the domain commands. Some endpoints (e.g. domain - /// availability) live behind a different host than the OAuth/`api_url` - /// service; this defaults to `api_url` when not overridden. Overridable - /// at runtime via `GDDY_DOMAINS_API_URL`. - #[env_config( - from_toml = parse_url_from_toml, - env = "DOMAINS_API_URL", - from_env = parse_url, - default_fn = default_domains_api_url - )] - pub domains_api_url: String, - - /// Base URL for the account management site (e.g. adding payment methods). - /// Defaults to `account.godaddy.com` for prod and `account.{env}-godaddy.com` - /// for other environments; overridable via `GDDY_ACCOUNT_URL` or local config. - #[env_config( - from_toml = parse_url_from_toml, - env = "ACCOUNT_URL", - from_env = parse_url, - default_fn = default_account_url - )] - pub account_url: String, -} - pub fn env_prefix(name: &str) -> String { name.to_uppercase().replace('-', "_") } -/// `name`'s `default_fn`: the field itself is never set by any real TOML/env -/// source, so this fires unconditionally, reading the environment's own -/// identity off the chain instead of a sibling field's value. -fn default_name(sources: &SourceChain<'_>) -> String { - sources.env_name().unwrap_or_default().to_owned() -} - -/// The already-resolved, cleaned `api_url` for this chain — same value the -/// `api_url` field itself holds, re-derived from the raw chain rather than -/// `Self` (a `default_fn` only ever sees the chain, not sibling fields -/// already computed on the struct being built). Declaring `api_url` before -/// the fields that call this, so their own resolution only runs once -/// `api_url`'s already succeeded, is what makes re-deriving from the raw -/// value safe — a malformed `api_url` fails the whole `assemble` before any -/// of these `default_fn`s could run. -fn current_api_url(sources: &SourceChain<'_>) -> String { - sources - .toml_value("api_url") - .and_then(toml::Value::as_str) - .and_then(clean_url) - .unwrap_or_default() -} - -fn default_auth_url(sources: &SourceChain<'_>) -> String { - derive_auth_url(¤t_api_url(sources)) -} - -fn default_token_url(sources: &SourceChain<'_>) -> String { - derive_token_url(¤t_api_url(sources)) -} - -fn default_domains_api_url(sources: &SourceChain<'_>) -> String { - current_api_url(sources) -} - -fn default_account_url(sources: &SourceChain<'_>) -> String { - derive_account_url(sources.env_name().unwrap_or_default()) -} - -fn derive_account_url(env_name: &str) -> String { - if env_name == "prod" { - return "https://account.godaddy.com".to_owned(); - } - substitute_env_host("https://account.godaddy.com", env_name) - .unwrap_or_else(|| "https://account.godaddy.com".to_owned()) -} - -/// Applies GoDaddy's internal-environment hostname convention -/// (`{env}-godaddy.com`) to a canonical `*.godaddy.com` URL, preserving any -/// subdomain prefix and the original scheme/path. Returns `None` if the -/// host isn't under `godaddy.com` — nothing to substitute. -fn substitute_env_host(url: &str, env_name: &str) -> Option { - let lower = url.to_ascii_lowercase(); - let scheme_len = if lower.starts_with("https://") { - 8 - } else if lower.starts_with("http://") { - 7 - } else { - return None; - }; - let rest = &url[scheme_len..]; - let host_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); - let host = &rest[..host_end]; - let host_lower = host.to_ascii_lowercase(); - let new_host = if host_lower == "godaddy.com" { - format!("{env_name}-godaddy.com") - } else { - let prefix = host_lower.strip_suffix(".godaddy.com")?; - format!("{}.{env_name}-godaddy.com", &host[..prefix.len()]) - }; - Some(format!( - "{}{}{}", - &url[..scheme_len], - new_host, - &rest[host_end..] - )) -} - -/// Resolve the environment-specific base URL for a catalog domain (used by -/// `api domain list` / `api call`). Checks an explicit override first — a -/// `_api_url` key from local config, or a `__API_URL` -/// env var read directly here (outside `GddyEnvConfig`'s own fields entirely, -/// since a per-domain override key isn't one of them). Checking the env var -/// directly makes this work uniformly for `prod`/`ote` builtins and custom -/// dev/test env names alike. Absent an override, falls back to GoDaddy's -/// `{env}-godaddy.com` hostname convention against `prod_base_url` for any -/// non-`prod` environment. -pub fn resolve_catalog_base_url(domain: &str, prod_base_url: &str, env_name: &str) -> String { - let override_key = format!("{}_api_url", domain.replace('-', "_")); - if let Some(url) = domain_override(&override_key, env_name, |k| std::env::var(k).ok()) { - return url; - } - if env_name == DEFAULT_ENV { - return prod_base_url.to_owned(); - } - substitute_env_host(prod_base_url, env_name).unwrap_or_else(|| prod_base_url.to_owned()) -} - -/// Pure lookup for [`resolve_catalog_base_url`]'s override, with the env-var -/// getter injected so tests stay parallel-safe (no real `std::env::set_var`). -/// `instance().source(env_name)` errors for an environment unknown to every -/// compiled/file layer, which this treats the same as "no local config -/// entry" — falling through to the env var — rather than propagating. -fn domain_override( - override_key: &str, - env_name: &str, - var: impl Fn(&str) -> Option, -) -> Option { - if let Ok(source) = instance().source(env_name) - && let Some(raw) = source - .toml_value(override_key) - .and_then(toml::Value::as_str) - && let Some(clean) = clean_url(raw) - { - return Some(clean); - } - let var_name = format!("{}_{}", env_prefix(env_name), override_key.to_uppercase()); - var(&var_name).and_then(|v| clean_url(&v)) -} - -fn derive_auth_url(api_url: &str) -> String { - format!("{}/v2/oauth2/authorize", api_url.trim_end_matches('/')) -} - -fn derive_token_url(api_url: &str) -> String { - format!("{}/v2/oauth2/token", api_url.trim_end_matches('/')) -} - -/// Validates and normalizes a candidate URL. Trims surrounding -/// whitespace and any trailing slash, and requires an `http(s)://` scheme with a -/// non-empty host (reqwest needs an absolute URL). Returns `None` for an -/// empty/whitespace or schemeless value, so a blank or malformed value never -/// resolves to a relative/unusable URL. -fn clean_url(raw: &str) -> Option { - let trimmed = raw.trim().trim_end_matches('/'); - // Require an http(s):// scheme (case-insensitive per RFC 3986, so `HTTPS://` - // is valid) and a non-empty host. The host is the segment before any - // path/query/fragment, so this rejects `https:///path`, `https://`, and - // `https://?x` (which a lenient URL parser would accept). - let lower = trimmed.to_ascii_lowercase(); - let scheme_len = if lower.starts_with("https://") { - "https://".len() - } else if lower.starts_with("http://") { - "http://".len() - } else { - return None; - }; - let host = trimmed[scheme_len..] - .split(['/', '?', '#']) - .next() - .unwrap_or(""); - (!host.is_empty()).then(|| trimmed.to_owned()) -} - -/// Base URL for the DevX Core API gateway for the given environment. -/// -/// Custom environments must set `_DEVX_CORE_URL` (for example, -/// `DEV_DEVX_CORE_URL`) or the global `DEVX_CORE_URL`. `prod` and `ote` use -/// their compiled-in endpoints unless either variable overrides them. -pub fn devx_core_url(name: &str) -> Option { - devx_core_url_with(name, |key| std::env::var(key).ok()) -} - -fn devx_core_url_with(name: &str, var: impl Fn(&str) -> Option) -> Option { - let prefix = env_prefix(name); - var(&format!("{prefix}_DEVX_CORE_URL")) - .and_then(|value| clean_url(&value)) - .or_else(|| var("DEVX_CORE_URL").and_then(|value| clean_url(&value))) - .or_else(|| { - BUILTIN_DEVX_CORE_URLS - .iter() - .find(|(n, _)| *n == name) - .map(|(_, url)| (*url).to_owned()) - }) -} - -/// Validates a candidate URL string. `EnvConfig` `from_env` shared by every -/// env-var-overridable URL field — an env var is already a plain `&str`, so -/// this is the direct validator with nothing to unwrap first. Blank values -/// never reach this — every `EnvConfig` field treats a blank source answer -/// as absent by default; a non-blank value must be a real http(s) URL. -fn parse_url(raw: &str) -> Result { - clean_url(raw).ok_or_else(|| format!("{raw:?} is not a valid http(s) URL")) -} - -/// `EnvConfig` `from_toml` shared by every URL field — a TOML value carries -/// its own type, so this checks it's actually a string before delegating to -/// [`parse_url`], the shared core every URL field's `from_env` also uses -/// directly. -fn parse_url_from_toml(value: &toml::Value) -> Result { - let raw = value - .as_str() - .ok_or_else(|| "expected a string".to_owned())?; - parse_url(raw) -} - /// Builds the compiled-in `ote`/`prod` `Environments`, shared by /// `CliConfig::with_environments`, `PkceAuthProvider::with_environments`, and /// this module's own resolution helpers below. @@ -396,45 +148,8 @@ pub fn is_known(name: &str) -> bool { #[cfg(test)] mod tests { use super::*; + use crate::environments::test_support::ENV_LOCK; use cli_engine::environments::EnvTable; - use std::sync::Mutex; - - // Serializes every test that touches real process env vars, so - // parallel test threads can't observe each other's GDDY_* overrides. - static ENV_LOCK: Mutex<()> = Mutex::new(()); - - /// RAII guard that sets an env var and restores it to its prior state on - /// drop — removing it if it wasn't already set, or putting the original - /// value back if it was — even if a test panics. Restoring rather than - /// unconditionally removing keeps a var a developer happens to already - /// have set in their shell from leaking into the rest of the test run. - struct EnvGuard { - key: &'static str, - prior: Option, - } - impl EnvGuard { - fn set(key: &'static str, value: &str) -> Self { - let prior = std::env::var(key).ok(); - // SAFETY: caller holds ENV_LOCK. - #[allow(unsafe_code)] - unsafe { - std::env::set_var(key, value); - } - Self { key, prior } - } - } - impl Drop for EnvGuard { - fn drop(&mut self) { - // SAFETY: caller holds ENV_LOCK; restore on any exit incl. panic. - #[allow(unsafe_code)] - unsafe { - match &self.prior { - Some(value) => std::env::set_var(self.key, value), - None => std::env::remove_var(self.key), - } - } - } - } #[test] fn register_scaffolds_a_file_only_environment() { @@ -510,355 +225,6 @@ auth_url = "not-a-url" assert!(err.to_string().contains("auth_url")); } - fn test_environment(name: &str, extend: impl FnOnce(EnvTable) -> EnvTable) -> GddyEnvConfig { - Environments::new(name) - .with_environment(name, extend(EnvTable::new())) - .resolve(name) - .expect("resolves") - } - - #[test] - fn resolved_env_derives_oauth_urls_from_api_url_when_unset() { - let resolved = test_environment("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - }); - assert_eq!(resolved.name, "dev"); - assert_eq!(resolved.api_url, "https://api.example.test"); - assert_eq!(resolved.client_id, "cid"); - assert_eq!( - resolved.auth_url, - "https://api.example.test/v2/oauth2/authorize" - ); - assert_eq!( - resolved.token_url, - "https://api.example.test/v2/oauth2/token" - ); - } - - #[test] - fn resolved_env_prefers_explicit_oauth_urls_over_derived() { - let resolved = test_environment("dev", |t| { - t.with("client_id", "cid") - .with("auth_url", "https://auth.example.test/authorize") - .with("token_url", "https://auth.example.test/token") - .with("api_url", "https://api.example.test") - }); - assert_eq!(resolved.auth_url, "https://auth.example.test/authorize"); - assert_eq!(resolved.token_url, "https://auth.example.test/token"); - } - - #[test] - fn resolved_env_falls_back_to_derived_oauth_urls_when_override_is_blank() { - // A blank `auth_url` (from a TOML value here, or from `GDDY_AUTH_URL=" "` - // — see `blank_env_var_override_falls_through_to_derived_auth_url`) is - // treated the same as unset. A genuinely malformed (non-blank) override - // is a hard resolve error instead — see - // `register_rejects_a_malformed_file_layer_auth_url_override_for_a_builtin`. - let resolved = test_environment("dev", |t| { - t.with("client_id", "cid") - .with("auth_url", " ") - .with("api_url", "https://api.example.test") - }); - assert_eq!( - resolved.auth_url, - "https://api.example.test/v2/oauth2/authorize" - ); - } - - #[test] - fn resolve_rejects_missing_api_url() { - let err = Environments::new("dev") - .with_environment("dev", EnvTable::new().with("client_id", "cid")) - .resolve::("dev") - .expect_err("no api_url"); - assert!(err.to_string().contains("api_url")); - } - - #[test] - fn resolve_rejects_missing_client_id() { - // client_id has no default: every environment (built-in, file, or - // hand-built for a test) must supply a real one. - let err = Environments::new("dev") - .with_environment( - "dev", - EnvTable::new().with("api_url", "https://api.example.test"), - ) - .resolve::("dev") - .expect_err("no client_id"); - assert!(err.to_string().contains("client_id")); - } - - #[test] - fn resolve_rejects_blank_api_url_final_value() { - // Unlike auth_url/token_url/domains_api_url/account_url, api_url has - // no sensible derived fallback, so blank is rejected (as `MissingField`, - // since a blank source answer is treated as absent by default). - let err = Environments::new("dev") - .with_environment( - "dev", - EnvTable::new() - .with("client_id", "cid") - .with("api_url", " "), - ) - .resolve::("dev") - .expect_err("blank api_url must be rejected"); - assert!(err.to_string().contains("api_url")); - } - - #[test] - fn domains_api_url_defaults_to_api_url() { - let resolved = test_environment("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - }); - assert_eq!(resolved.domains_api_url, resolved.api_url); - } - - #[test] - fn domains_api_url_override_is_respected() { - let resolved = test_environment("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - .with("domains_api_url", "https://domains.example.test") - }); - assert_eq!(resolved.domains_api_url, "https://domains.example.test"); - } - - #[test] - fn account_url_defaults_to_bare_domain_for_prod() { - let resolved = test_environment("prod", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.godaddy.com") - }); - assert_eq!(resolved.account_url, "https://account.godaddy.com"); - } - - #[test] - fn account_url_defaults_to_prefixed_domain_for_non_prod() { - let resolved = test_environment("ote", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.ote-godaddy.com") - }); - assert_eq!(resolved.account_url, "https://account.ote-godaddy.com"); - } - - #[test] - fn account_url_override_is_respected() { - let resolved = test_environment("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - .with("account_url", "https://account.override.test") - }); - assert_eq!(resolved.account_url, "https://account.override.test"); - } - - fn test_environment_with_app_id( - name: &str, - extend: impl FnOnce(EnvTable) -> EnvTable, - ) -> GddyEnvConfig { - Environments::new(name) - .with_app_id(APP_ID) - .with_environment(name, extend(EnvTable::new())) - .resolve(name) - .expect("resolves") - } - - #[test] - fn env_var_overrides_auth_url() { - let _g = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _guard = EnvGuard::set("GDDY_AUTH_URL", "https://auth.override.test"); - - let resolved = test_environment_with_app_id("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - .with("auth_url", "https://auth.example.test/authorize") - }); - assert_eq!( - resolved.auth_url, "https://auth.override.test", - "env var must win over the TOML value" - ); - } - - #[test] - fn env_var_overrides_token_url() { - let _g = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _guard = EnvGuard::set("GDDY_TOKEN_URL", "https://token.override.test"); - - let resolved = test_environment_with_app_id("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - }); - assert_eq!(resolved.token_url, "https://token.override.test"); - } - - #[test] - fn env_var_overrides_domains_api_url() { - let _g = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _guard = EnvGuard::set("GDDY_DOMAINS_API_URL", "https://domains.override.test"); - - let resolved = test_environment_with_app_id("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - }); - assert_eq!(resolved.domains_api_url, "https://domains.override.test"); - } - - #[test] - fn env_var_overrides_account_url() { - let _g = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _guard = EnvGuard::set("GDDY_ACCOUNT_URL", "https://account.override.test"); - - let resolved = test_environment_with_app_id("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - }); - assert_eq!(resolved.account_url, "https://account.override.test"); - } - - #[test] - fn substitute_env_host_prefixes_a_bare_domain() { - assert_eq!( - substitute_env_host("https://godaddy.com", "ote"), - Some("https://ote-godaddy.com".to_owned()) - ); - } - - #[test] - fn substitute_env_host_preserves_subdomain_and_path() { - assert_eq!( - substitute_env_host( - "https://fulfillment.api.commerce.godaddy.com/v1/commerce", - "dev" - ), - Some("https://fulfillment.api.commerce.dev-godaddy.com/v1/commerce".to_owned()) - ); - } - - #[test] - fn substitute_env_host_returns_none_for_non_godaddy_host() { - assert_eq!(substitute_env_host("https://example.com/v1", "ote"), None); - } - - #[test] - fn domain_override_falls_back_to_env_var_when_no_local_config_entry() { - // "fulfillments-catalog-test" is not a compiled builtin and has no - // local config entry, so the first (local-config) branch misses and - // the injected var getter is consulted directly. - let var = |k: &str| { - (k == "FULFILLMENTS_CATALOG_TEST_FULFILLMENTS_API_URL") - .then(|| "https://fulfillments.example.test".to_owned()) - }; - let resolved = domain_override("fulfillments_api_url", "fulfillments-catalog-test", var); - assert_eq!( - resolved, - Some("https://fulfillments.example.test".to_owned()) - ); - } - - #[test] - fn domain_override_is_none_when_neither_layer_has_it() { - let resolved = domain_override("fulfillments_api_url", "fulfillments-catalog-test", |_| { - None - }); - assert_eq!(resolved, None); - } - - #[test] - fn resolve_catalog_base_url_returns_prod_unchanged() { - let url = resolve_catalog_base_url( - "fulfillments", - "https://fulfillment.api.commerce.godaddy.com/v1/commerce", - "prod", - ); - assert_eq!( - url, - "https://fulfillment.api.commerce.godaddy.com/v1/commerce" - ); - } - - #[test] - fn resolve_catalog_base_url_applies_convention_for_non_prod() { - // No override exists anywhere for this made-up env/domain pair, so - // this exercises the `{env}-godaddy.com` convention fallback. - let url = resolve_catalog_base_url( - "fulfillments", - "https://fulfillment.api.commerce.godaddy.com/v1/commerce", - "ote", - ); - assert_eq!( - url, - "https://fulfillment.api.commerce.ote-godaddy.com/v1/commerce" - ); - } - - #[test] - fn env_var_override_rejects_a_malformed_url() { - let _g = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _guard = EnvGuard::set("GDDY_AUTH_URL", "not-a-url"); - - let err = Environments::new("dev") - .with_app_id(APP_ID) - .with_environment( - "dev", - EnvTable::new() - .with("client_id", "cid") - .with("api_url", "https://api.example.test"), - ) - .resolve::("dev") - .expect_err("a malformed env var override must be a hard error"); - assert!(err.to_string().contains("auth_url")); - } - - #[test] - fn blank_env_var_override_falls_through_to_derived_auth_url() { - let _g = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _guard = EnvGuard::set("GDDY_AUTH_URL", " "); - - let resolved = test_environment_with_app_id("dev", |t| { - t.with("client_id", "cid") - .with("api_url", "https://api.example.test") - }); - assert_eq!( - resolved.auth_url, "https://api.example.test/v2/oauth2/authorize", - "a blank env var override is treated as absent, same as a blank TOML value" - ); - } - - #[test] - fn clean_url_requires_a_non_empty_host() { - assert_eq!( - clean_url("https://api.example.test/"), - Some("https://api.example.test".to_owned()) - ); - assert!(clean_url("https:///path").is_none()); - assert!(clean_url("https://").is_none()); - assert!(clean_url("https://?x").is_none()); - assert!(clean_url("ftp://x").is_none()); - assert!(clean_url("api.example.test").is_none()); - assert!(clean_url("not a url").is_none()); - assert_eq!( - clean_url("HTTPS://api.Example.test"), - Some("HTTPS://api.Example.test".to_owned()) - ); - assert_eq!( - clean_url("http://localhost:8080/api/"), - Some("http://localhost:8080/api".to_owned()) - ); - } - #[test] fn env_prefix_uppercases_and_replaces_hyphen() { assert_eq!(env_prefix("ote"), "OTE"); @@ -915,45 +281,4 @@ auth_url = "not-a-url" let envs = resolve_default_environments("prod"); assert_eq!(envs.default_env(), "prod"); } - - #[test] - fn devx_core_url_uses_prod_and_ote_builtins() { - assert_eq!( - devx_core_url_with("prod", |_| None).as_deref(), - Some("https://api.developer.commerce.godaddy.com") - ); - assert_eq!( - devx_core_url_with("ote", |_| None).as_deref(), - Some("https://api.developer.commerce.ote-godaddy.com") - ); - } - - #[test] - fn devx_core_url_global_override_wins() { - assert_eq!( - devx_core_url_with("prod", |key| { - (key == "DEVX_CORE_URL").then(|| " http://localhost:4000/ ".to_owned()) - }) - .as_deref(), - Some("http://localhost:4000") - ); - } - - #[test] - fn devx_core_url_per_environment_override_wins_over_global() { - assert_eq!( - devx_core_url_with("dev", |key| match key { - "DEV_DEVX_CORE_URL" => Some("https://dev-core.example.test/".to_owned()), - "DEVX_CORE_URL" => Some("https://shared-core.example.test".to_owned()), - _ => None, - }) - .as_deref(), - Some("https://dev-core.example.test") - ); - } - - #[test] - fn devx_core_url_custom_env_requires_override() { - assert_eq!(devx_core_url_with("dev", |_| None), None); - } } diff --git a/rust/src/environments/test_support.rs b/rust/src/environments/test_support.rs new file mode 100644 index 00000000..57e63dcf --- /dev/null +++ b/rust/src/environments/test_support.rs @@ -0,0 +1,43 @@ +//! Shared test-only helpers for env-var-touching tests across this module's +//! submodules — see [`ENV_LOCK`]'s own doc for why serialization is needed. + +use std::sync::Mutex; + +// Serializes every test that touches real process env vars, so parallel +// test threads can't observe each other's GDDY_* overrides. +pub(super) static ENV_LOCK: Mutex<()> = Mutex::new(()); + +/// RAII guard that sets an env var and restores it to its prior state on +/// drop — removing it if it wasn't already set, or putting the original +/// value back if it was — even if a test panics. Restoring rather than +/// unconditionally removing keeps a var a developer happens to already +/// have set in their shell from leaking into the rest of the test run. +pub(super) struct EnvGuard { + key: &'static str, + prior: Option, +} + +impl EnvGuard { + pub(super) fn set(key: &'static str, value: &str) -> Self { + let prior = std::env::var(key).ok(); + // SAFETY: caller holds ENV_LOCK. + #[allow(unsafe_code)] + unsafe { + std::env::set_var(key, value); + } + Self { key, prior } + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + // SAFETY: caller holds ENV_LOCK; restore on any exit incl. panic. + #[allow(unsafe_code)] + unsafe { + match &self.prior { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), + } + } + } +} From c87906f193772869cf271efd6f3b2c15efeef08b Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Mon, 17 Aug 2026 13:08:12 -0700 Subject: [PATCH 2/2] fix(environments): address Copilot review feedback on split Make the ENV_LOCK comment a real rustdoc doc comment so the module doc's cross-reference to "ENV_LOCK's own doc" actually resolves, and hold ENV_LOCK in the two catalog.rs tests that indirectly read real process env vars through resolve_catalog_base_url, matching this module's existing test-serialization convention. Co-Authored-By: Claude Sonnet 5 --- rust/src/environments/catalog.rs | 13 +++++++++++++ rust/src/environments/test_support.rs | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/rust/src/environments/catalog.rs b/rust/src/environments/catalog.rs index 6302a449..0a5460f8 100644 --- a/rust/src/environments/catalog.rs +++ b/rust/src/environments/catalog.rs @@ -51,6 +51,7 @@ fn domain_override( #[cfg(test)] mod tests { use super::*; + use crate::environments::test_support::ENV_LOCK; #[test] fn domain_override_falls_back_to_env_var_when_no_local_config_entry() { @@ -78,6 +79,13 @@ mod tests { #[test] fn resolve_catalog_base_url_returns_prod_unchanged() { + // `resolve_catalog_base_url` reads real process env vars (via + // `domain_override`'s injected `std::env::var`), so it must be + // serialized against tests elsewhere in this module family that + // mutate them with `EnvGuard`/`set_var` (see `ENV_LOCK`'s own doc). + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let url = resolve_catalog_base_url( "fulfillments", "https://fulfillment.api.commerce.godaddy.com/v1/commerce", @@ -91,6 +99,11 @@ mod tests { #[test] fn resolve_catalog_base_url_applies_convention_for_non_prod() { + // See `resolve_catalog_base_url_returns_prod_unchanged` for why this + // takes `ENV_LOCK`. + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); // No override exists anywhere for this made-up env/domain pair, so // this exercises the `{env}-godaddy.com` convention fallback. let url = resolve_catalog_base_url( diff --git a/rust/src/environments/test_support.rs b/rust/src/environments/test_support.rs index 57e63dcf..a3f3377e 100644 --- a/rust/src/environments/test_support.rs +++ b/rust/src/environments/test_support.rs @@ -3,8 +3,8 @@ use std::sync::Mutex; -// Serializes every test that touches real process env vars, so parallel -// test threads can't observe each other's GDDY_* overrides. +/// Serializes every test that touches real process env vars, so parallel +/// test threads can't observe each other's GDDY_* overrides. pub(super) static ENV_LOCK: Mutex<()> = Mutex::new(()); /// RAII guard that sets an env var and restores it to its prior state on